Thread overview
[Bug 147] GDC emits unnecessary dead struct inits
Jul 31, 2014
Iain Buclaw
Jul 31, 2014
Johannes Pfau
Jul 31, 2014
Johannes Pfau
Jun 10, 2017
Iain Buclaw
Jun 11, 2017
Iain Buclaw
July 31, 2014
http://bugzilla.gdcproject.org/show_bug.cgi?id=147

--- Comment #1 from Iain Buclaw <ibuclaw@gdcproject.org> ---
The codegen is:

S a;              // memset(&a, 0, S.sizeof)
a.v = argv.length // a.v = argv.length

This first step is no easy to get around, as it is supposed to fill out any alignment holes in the struct.

This itself ensures that 'assert(a == b)' is true in most cases (uses memcmp).


I do recognise that:

1) It's generally bad to use a == b to compare structs
2) Structs that return in registers have their 0'd alignment holes destroyed
with garbage.


Perhaps we shouldn't care about filling alignment holes, but that would break the autotester (last time I checked), and maybe some code that relies on memcmp'ing structs for equality.

-- 
You are receiving this mail because:
You are watching all bug changes.


July 31, 2014
http://bugzilla.gdcproject.org/show_bug.cgi?id=147

art.08.09@gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |art.08.09@gmail.com

--- Comment #2 from art.08.09@gmail.com ---
I reported this because I was suspecting it was a symptom of some frontend+glue problem, which was preventing the dead code from being removed. But I've now checked what GCC does for similar 'C' cases and apparently the problem exists there too, it's just not as visible in 'C' because of the lack of default init. So this might be an upstream issue and could be closed, if you think there isn't anything that could be done about it in GDC.

Because of the default initialization in D and frequent use of structs for creating new types, the impact of this missed optimization is much larger than for C. For example, a bounds-checked int implementation becomes more expensive because of this.

I tried a few other things, but was not able to find a workaround. Eg

-------------------------------------------------------
void main(string[] argv) {
   S a = void;

   import gcc.builtins;
   __builtin_memset(&a, 0, S.sizeof);

   a.v = argv.length; /* Modifies the whole struct. */

   if (a.v==42)
      f();
}
-------------------------------------------------------

In this case the memset *is* completely eliminated. But now the unnecessary a.v store isn't removed...

-- 
You are receiving this mail because:
You are watching all bug changes.


July 31, 2014
http://bugzilla.gdcproject.org/show_bug.cgi?id=147

Johannes Pfau <johannespfau@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |johannespfau@gmail.com

--- Comment #3 from Johannes Pfau <johannespfau@gmail.com> ---
Yes, this probably is a GCC bug. I think Iain was referring to http://bugzilla.gdcproject.org/show_bug.cgi?id=57 in his reply, if you want to read more about that. #57 could also be a bug with might be better to fix in GCC.

-- 
You are receiving this mail because:
You are watching all bug changes.


July 31, 2014
http://bugzilla.gdcproject.org/show_bug.cgi?id=147

Johannes Pfau <johannespfau@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |http://bugzilla.gdcproject.
                   |                            |org/show_bug.cgi?id=57

-- 
You are receiving this mail because:
You are watching all bug changes.


June 10, 2017
https://bugzilla.gdcproject.org/show_bug.cgi?id=147

Iain Buclaw <ibuclaw@gdcproject.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #4 from Iain Buclaw <ibuclaw@gdcproject.org> ---
>From gdc-5 and onwards, I'm not able to reproduce this.

https://explore.dgnu.org/g/ENvKE5

As per last comment, it likely was a missed optimization in gcc itself.

-- 
You are receiving this mail because:
You are watching all bug changes.
June 11, 2017
https://bugzilla.gdcproject.org/show_bug.cgi?id=147

Iain Buclaw <ibuclaw@gdcproject.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

-- 
You are receiving this mail because:
You are watching all bug changes.