[Bug tree-optimization/36602] memset should be optimized into an empty CONSTRUCTOR

[Bug tree-optimization/36602] memset should be optimized into an empty CONSTRUCTOR

[ak at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36602

ak at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ak at gcc dot gnu.org,
                   |                            |mjambor at suse dot cz

--- Comment #3 from ak at gcc dot gnu.org 2011-06-22 20:30:56 UTC ---
I ran into a similar problem in my code.

It would be nice if memset didn't break SRA.

[Bug tree-optimization/36602] memset should be optimized into an empty CONSTRUCTOR

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36602

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         Depends on|                            |16427
         AssignedTo|unassigned at gcc dot       |rguenth at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #4 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-06-06 10:20:05 UTC ---
The memset DSE is PR16427.

Still we should represent this as a = {}; on the tree level, thus re-confirmed.
And mine.

[Bug tree-optimization/36602] memset should be optimized into an empty CONSTRUCTOR

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36602

--- Comment #5 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-06-06 14:11:07 UTC ---
Created attachment 27563
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27563
proposed patch

Not as easy when the initialized struct contains padding ...

The following testcase is reduced from a fail of
gcc.target/x86_64/abi/test_struct_returning.c
where SRA messes up information in the padding.  With s = {} being
a memset SRA would not do anything.

The question in general would be whether memcpy/memset affect padding
in a well-defined way, thus, if memset (&s, 0, sizeof (s)) is equivalent
to s = {} and if memcpy (&s, &r, sizeof (s)) is equivalent to s = r.
We at least assume that in the way we currently fold memcpy.


struct S { char c; long long i; };

struct S __attribute__((noinline,noclone))
foo (void)
{
  struct S s = {};
  s.c = 42;
  return s;
}

void __attribute__((noinline,noclone))
keepit (struct S *s)
{
  __builtin_memset (s, -1, sizeof (struct S));
}

void __attribute__((noinline,noclone))
clobberstack ()
{
  struct S s;
  keepit (&s);
}

int main ()
{
  struct S s1, s2 = { 42, 0 };
  clobberstack();
  s1 = foo();
  if (__builtin_memcmp (&s1, &s2, sizeof (struct S)) != 0)
    __builtin_abort ();
  return 0;
}

[Bug tree-optimization/36602] memset should be optimized into an empty CONSTRUCTOR

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36602

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jason at gcc dot gnu.org,
                   |                            |jsm28 at gcc dot gnu.org

--- Comment #6 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-06-06 14:13:15 UTC ---
Joseph, Jason - any opinion on the question of equivalence between
memcpy/memset and an aggregate assignment/init (also consider anonymous
memory)?

[Bug tree-optimization/36602] memset should be optimized into an empty CONSTRUCTOR

[jason at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36602

--- Comment #7 from Jason Merrill <jason at gcc dot gnu.org> 2012-06-06 16:36:31 UTC ---
(In reply to comment #6)
> Joseph, Jason - any opinion on the question of equivalence between
> memcpy/memset and an aggregate assignment/init (also consider anonymous
> memory)?

The transformation seems safe to me.  Aggregate assignment/init leave anonymous
bit-fields and padding with unspecified contents, but I imagine GCC
initializes/copies them anyway.  In C++ any class that would be affected by the
difference is not trivially copyable, so using memset/memcpy is already
undefined behavior.

[Bug tree-optimization/36602] memset should be optimized into an empty CONSTRUCTOR

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36602

--- Comment #8 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-06-11 10:58:41 UTC ---
Ok.  I suppose I need to "fix" gcc.target/x86_64/abi/test_struct_returning.c
somehow then ... :/

[Bug tree-optimization/36602] memset should be optimized into an empty CONSTRUCTOR

[andi-gcc at firstfloor dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=36602

Andi Kleen <andi-gcc at firstfloor dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |andi-gcc at firstfloor dot org

--- Comment #9 from Andi Kleen <andi-gcc at firstfloor dot org> ---
Any progress on fixing the test case, so that this can be finally fixed?

[Bug tree-optimization/36602] memset should be optimized into an empty CONSTRUCTOR

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=36602

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Andi Kleen from comment #9)
> Any progress on fixing the test case, so that this can be finally fixed?

I have no idea how to do that without making the testcase test sth different.
We could of course simply turn off SRA for it ...

That said, the difference the optimization may cause might be surprising to
people.  That is, that the following may abort()

struct f { char c; int i; };
struct f2 { int i; int j; };
int main()
{
  struct f f;
  struct f2 f2;
  memset(&f, 0, sizeof (f));
  memcpy(&f2, f, sizeof (f));
  if (memcmp (&f, &f2) != 0)
   abort ();
  return 0;
}

because suddenly the semantics of a memset and a memcpy depend on the
type (_what_ type?!) of the pointed-to type.

Note we can "simply" circumvent the issue by using a char[n] typed
CONSTRUCTOR / type for the copy.  Sth I wanted to followup with
now that the folding happens always on GIMPLE.

It seems to me that SRA may not simply omit padding for aggregate
copies if it cannot prove that the padding is not used.  At least
on GIMPLE the semantics of an aggregate assignment is that of a
memmove, not of a field-wise copy.  Martin - can you look into this
wrong-code bug?  Basically if SRA decides it needs to re-materialize
the object (here before the memcmp call) then it has to re-materialize
padding as well.

[Bug tree-optimization/36602] memset should be optimized into an empty CONSTRUCTOR

[jamborm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=36602

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jamborm at gcc dot gnu.org

--- Comment #13 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Memset is not being turned into a constructor but early DSE eliminates it now
anyway and then the testcase is optimized just into return 0;.  Do we want to
keep this bug opened?

(Issues with SRA padding are also tracked elsewhere.)

[Bug tree-optimization/36602] memset should be optimized into an empty CONSTRUCTOR

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=36602

--- Comment #14 from Richard Biener <rguenth at gcc dot gnu.org> ---
Yes, I think so.  = {} has the advantage that the destination isn't
address-taken compared to memset which has alias analysis benefits.

[Bug tree-optimization/36602] memset should be optimized into an empty CONSTRUCTOR

[crrodriguez at opensuse dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=36602

--- Comment #15 from Cristian Rodríguez <crrodriguez at opensuse dot org> ---
Was this issue ever fixed/addressed?

[Bug tree-optimization/36602] memset should be optimized into an empty CONSTRUCTOR

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=36602

--- Comment #16 from Richard Biener <rguenth at gcc dot gnu.org> ---
No.  The fallout was never resolved.

[Bug tree-optimization/36602] memset should be optimized into an empty CONSTRUCTOR

[pinskia at gcc dot gnu dot org]

------- Comment #2 from pinskia at gcc dot gnu dot org  2010-03-02 18:52 -------
We do look through the memset at the tree level now but we don't remove it
still.  


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|TREE                        |
   Last reconfirmed|2008-06-22 21:26:09         |2010-03-02 18:52:12
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36602

[Bug tree-optimization/36602] memset should be optimized into an empty CONSTRUCTOR

[rguenth at gcc dot gnu dot org]

------- Comment #1 from rguenth at gcc dot gnu dot org  2008-06-22 21:26 -------
Confirmed.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2008-06-22 21:26:09
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36602