[Bug middle-end/94103] New: Wrong optimization: reading value of a variable changes its representation for optimizer

[Bug middle-end/94103] New: Wrong optimization: reading value of a variable changes its representation for optimizer

[ch3root at openwall dot com]

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

            Bug ID: 94103
           Summary: Wrong optimization: reading value of a variable
                    changes its representation for optimizer
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ch3root at openwall dot com
  Target Milestone: ---

It seems the optimizer sometimes computes the representation of variables from
its value instead of tracking it directly. This is wrong when the value admits
different representations.
(Given that the value is used, the representation should be valid (non-trap).)

Example with lost padding in x86-64 long double:

----------------------------------------------------------------------
#include <string.h>
#include <stdio.h>

int main()
{
    long double x;

    // fill x including the padding
    unsigned long u[2] = {0xEEEEEEEEEEEEEEEE, 0xEEEEEEEEEEEEEEEE};
    memcpy(&x, &u, sizeof x);

    // print the representation of x (initial)
    memcpy(&u, &x, sizeof u);
    printf("%016lX %016lX\n", u[1], u[0]);

    // change the representation of x a bit
    ++*(unsigned char *)&x;
    (void)-x; // use the value of x but don't write it

    // print the representation of x (resulting)
    memcpy(&u, &x, sizeof u);
    printf("%016lX %016lX\n", u[1], u[0]);
}
----------------------------------------------------------------------
$ gcc -std=c2x -pedantic -Wall -Wextra test.c && ./a.out
EEEEEEEEEEEEEEEE EEEEEEEEEEEEEEEE
EEEEEEEEEEEEEEEE EEEEEEEEEEEEEEEF
$ gcc -std=c2x -pedantic -Wall -Wextra -O3 test.c && ./a.out
EEEEEEEEEEEEEEEE EEEEEEEEEEEEEEEE
000000000000EEEE EEEEEEEEEEEEEEEF
----------------------------------------------------------------------
gcc x86-64 version: gcc (GCC) 10.0.1 20200305 (experimental)
----------------------------------------------------------------------

Zeros in the last output line are wrong.

[Bug middle-end/94103] Wrong optimization: reading value of a variable changes its representation for optimizer

[ch3root at openwall dot com]

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

--- Comment #1 from Alexander Cherepanov <ch3root at openwall dot com> ---
Example with decimal floating-point:

----------------------------------------------------------------------
#include <string.h>
#include <stdio.h>

int main()
{
    _Decimal32 x = 9999999; // maximum significand
    unsigned u;

    memcpy(&u, &x, sizeof u);
    printf("%#08x\n", u);

    ++*(unsigned char *)&x; // create non-canonical representation of 0
    (void)-x;

    memcpy(&u, &x, sizeof u);
    printf("%#08x\n", u);
}
----------------------------------------------------------------------
$ gcc -std=c2x -pedantic -Wall -Wextra test.c && ./a.out
0x6cb8967f
0x6cb89680
$ gcc -std=c2x -pedantic -Wall -Wextra -O3 test.c && ./a.out
0x6cb8967f
0x32800000
----------------------------------------------------------------------
gcc x86-64 version: gcc (GCC) 10.0.1 20200305 (experimental)
----------------------------------------------------------------------

Unoptimized results are right, optimized -- wrong.

[Bug middle-end/94103] Wrong optimization: reading value of a variable changes its representation for optimizer

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
long double has padding bits on x86_64 so that is not shocking there.

_Decimal3 is a different issue all together.

[Bug middle-end/94103] Wrong optimization: reading value of a variable changes its representation for optimizer

[ch3root at openwall dot com]

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

--- Comment #3 from Alexander Cherepanov <ch3root at openwall dot com> ---
*** Bug 92824 has been marked as a duplicate of this bug. ***

[Bug target/94103] Wrong optimization: reading value of a variable changes its representation for optimizer

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|middle-end                  |target
           Keywords|                            |wrong-code
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2020-03-10
             Target|                            |x86_64-*-*, i?86-*-*,
                   |                            |m68k-*-*
                 CC|                            |law at gcc dot gnu.org,
                   |                            |rguenth at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
There's a related bug about x87 stores not storing all bytes which was closed
as INVALID, the testcase was using a union to pun long double and a character
array IIRC.  Here the situation is quite similar - the IL has (from your
unused load):

  x.1_6 = x;
  _18 = MEM[(char * {ref-all})&x];
  MEM[(char * {ref-all})&u] = _18;
  _7 = u[0];
  _8 = u[1];
  printf ("%016lX %016lX\n", _8, _7);

where x.1_6 is of type long double and _18 is __int128.  Value-numbering
then decides that it can elide the load to _18 and also optimize the loads
from u[] as:

  x.1_6 = x;
  _31 = VIEW_CONVERT_EXPR<__int128 unsigned>(x.1_6);
  MEM[(char * {ref-all})&u] = _31;
  _32 = (long unsigned int) _31;
  _33 = BIT_FIELD_REF <_31, 64, 64>;
  printf ("%016lX %016lX\n", _33, _32);

which is because the backend tells us the FP load x.1_6 = x loads all
bits and do not modify the underlying representation.  Now, for x87
modes GET_MODE_SIZE isn't in agreement with what the actual instruction
does nor does the load reflect the fact that a x87 load (not the
long double variant) can end up doing a rounding step.  m68k is
probably similarly affected.  And yes, compared to the other bug I
was able to close as INVALID conveniently this one looks "real"
(if also artificially constructed and unfortunate...).  Jeff, you
also were involved in the other bug, do you agree here?  I still don't
see any good solution though.

I agree that the decimal float variant is an entirely different bug,
maybe you can open a new one for this?

[Bug target/94103] Wrong optimization: reading value of a variable changes its representation for optimizer

[ch3root at openwall dot com]

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

Alexander Cherepanov <ch3root at openwall dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=94122

--- Comment #5 from Alexander Cherepanov <ch3root at openwall dot com> ---
(In reply to Richard Biener from comment #4)
> I agree that the decimal float variant is an entirely different bug,
> maybe you can open a new one for this?

Sure -- pr94122.

[Bug target/94103] Wrong optimization: reading value of a variable changes its representation for optimizer

[law at redhat dot com]

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

Jeffrey A. Law <law at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |law at redhat dot com

--- Comment #6 from Jeffrey A. Law <law at redhat dot com> ---
It was pr93270.

I agree.  It feels like something should be looking at the TYPE_PRECISION that
is instead looking at TYPE_SIZE.

[Bug target/94103] Wrong optimization: reading value of a variable changes its representation for optimizer

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org
             Status|NEW                         |ASSIGNED
      Known to work|                            |10.0

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
commit 1dc00a8ec9aeba86b74b16bff6f171824bb7b4a1 (HEAD -> master, origin/master,
origin/HEAD)
Author: Richard Biener <rguenther@suse.de>
Date:   Thu Mar 12 14:18:35 2020 +0100

    tree-optimization/94103 avoid CSE of loads with padding

    VN currently replaces a load of a 16 byte entity 128 bits of precision
    (TImode) with the result of a load of a 16 byte entity with 80 bits of
    mode precision (XFmode).  That will go downhill since if the padding
    bits are not actually filled with memory contents those bits are
    missing.

    2020-03-12  Richard Biener  <rguenther@suse.de>

            PR tree-optimization/94103
            * tree-ssa-sccvn.c (visit_reference_op_load): Avoid type
            punning when the mode precision is not sufficient.

            * gcc.target/i386/pr94103.c: New testcase.

[Bug target/94103] Wrong optimization: reading value of a variable changes its representation for optimizer

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |9.3.0

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
The old VN doesn't value-number unused defs so the actual testcase doesn't fail
there.  Still broken with GCC 9 though.  Adjusted testcases that actually
use the FP load would trigger with older compilers as well.

[Bug target/94103] Wrong optimization: reading value of a variable changes its representation for optimizer

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

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

--- Comment #9 from Martin Liška <marxin at gcc dot gnu.org> ---
commit r10-7145-g1dc00a8ec9aeba86b74b16bff6f171824bb7b4a1
Author: Richard Biener <rguenther@suse.de>
Date:   Thu Mar 12 14:18:35 2020 +0100

    tree-optimization/94103 avoid CSE of loads with padding

    VN currently replaces a load of a 16 byte entity 128 bits of precision
    (TImode) with the result of a load of a 16 byte entity with 80 bits of
    mode precision (XFmode).  That will go downhill since if the padding
    bits are not actually filled with memory contents those bits are
    missing.

    2020-03-12  Richard Biener  <rguenther@suse.de>

            PR tree-optimization/94103
            * tree-ssa-sccvn.c (visit_reference_op_load): Avoid type
            punning when the mode precision is not sufficient.

            * gcc.target/i386/pr94103.c: New testcase.

[Bug target/94103] Wrong optimization: reading value of a variable changes its representation for optimizer

[ch3root at openwall dot com]

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

--- Comment #10 from Alexander Cherepanov <ch3root at openwall dot com> ---
The case of assignment+memcpy -- testcases in comment 0, in pr92824 and similar
-- is fixed.

But the case of memset+assignment -- pr93270 and pr61872 (these seem to be
dups) -- is not fixed. Is it supposed to be fixed?

Before, I've seen somewhat contradicting approaches in bug 92486, comment 12,
which says that memset+assignment should set padding in structs, and in bug
93270, comment 4, which implies that memset+assignment shouldn't set padding in
long double. I'm in no way trying to imply that memset+assignment should or
shouldn't be fixed, just wondering if there is a difference of two cases.

[Bug target/94103] Wrong optimization: reading value of a variable changes its representation for optimizer

[rguenther at suse dot de]

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

--- Comment #11 from rguenther at suse dot de <rguenther at suse dot de> ---
On Fri, 20 Mar 2020, ch3root at openwall dot com wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94103
> 
> --- Comment #10 from Alexander Cherepanov <ch3root at openwall dot com> ---
> The case of assignment+memcpy -- testcases in comment 0, in pr92824 and similar
> -- is fixed.
> 
> But the case of memset+assignment -- pr93270 and pr61872 (these seem to be
> dups) -- is not fixed. Is it supposed to be fixed?
> 
> Before, I've seen somewhat contradicting approaches in bug 92486, comment 12,
> which says that memset+assignment should set padding in structs, and in bug
> 93270, comment 4, which implies that memset+assignment shouldn't set padding in
> long double. I'm in no way trying to imply that memset+assignment should or
> shouldn't be fixed, just wondering if there is a difference of two cases.

I think if the user writes a long double store then padding becomes
undefined so the testcase in comment#1 in PR61872 is technically
undefined IMHO.

[Bug target/94103] Wrong optimization: reading value of a variable changes its representation for optimizer

[schwab@linux-m68k.org]

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

--- Comment #12 from Andreas Schwab <schwab@linux-m68k.org> ---
The values of any padding bits are unspecified.

[Bug target/94103] Wrong optimization: reading value of a variable changes its representation for optimizer

[ch3root at openwall dot com]

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

--- Comment #13 from Alexander Cherepanov <ch3root at openwall dot com> ---
(In reply to rguenther@suse.de from comment #11)
> I think if the user writes a long double store then padding becomes
> undefined so the testcase in comment#1 in PR61872 is technically
> undefined IMHO.

Sure, but why not the same for structs?

[Bug target/94103] Wrong optimization: reading value of a variable changes its representation for optimizer

[rguenther at suse dot de]

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

--- Comment #14 from rguenther at suse dot de <rguenther at suse dot de> ---
On March 20, 2020 5:29:24 PM GMT+01:00, ch3root at openwall dot com
<gcc-bugzilla@gcc.gnu.org> wrote:
>https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94103
>
>--- Comment #13 from Alexander Cherepanov <ch3root at openwall dot com>
>---
>(In reply to rguenther@suse.de from comment #11)
>> I think if the user writes a long double store then padding becomes
>> undefined so the testcase in comment#1 in PR61872 is technically
>> undefined IMHO.
>
>Sure, but why not the same for structs?

>From a language Pov that's the same.
But memcpy and friends work on any dynamic type so have to copy all bytes.

[Bug target/94103] Wrong optimization: reading value of a variable changes its representation for optimizer

[ch3root at openwall dot com]

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

--- Comment #15 from Alexander Cherepanov <ch3root at openwall dot com> ---
(In reply to rguenther@suse.de from comment #14)
> From a language Pov that's the same.
> But memcpy and friends work on any dynamic type so have to copy all bytes.
Sorry, I don't understand. Bug 61872, comment 1, and bug 92486, comment 9, look
the same -- a memset followed by an assignment.  But the former is with long
double while the latter is with a struct.

You mean that assignments of structs are equivalent to memcpy while assignments
of long doubles aren't? Why the difference?

[Bug target/94103] Wrong optimization: reading value of a variable changes its representation for optimizer

[rguenther at suse dot de]

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

--- Comment #16 from rguenther at suse dot de <rguenther at suse dot de> ---
On Fri, 20 Mar 2020, ch3root at openwall dot com wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94103
> 
> --- Comment #15 from Alexander Cherepanov <ch3root at openwall dot com> ---
> (In reply to rguenther@suse.de from comment #14)
> > From a language Pov that's the same.
> > But memcpy and friends work on any dynamic type so have to copy all bytes.
> Sorry, I don't understand. Bug 61872, comment 1, and bug 92486, comment 9, look
> the same -- a memset followed by an assignment.  But the former is with long
> double while the latter is with a struct.
> 
> You mean that assignments of structs are equivalent to memcpy while assignments
> of long doubles aren't? Why the difference?

No, assignment of structs from a language POV are not equivalent to 
memcpy.  For those the issue is that GCC thinks they are (and forever
did that).  Which means for the GCC middle-end (GIMPLE) assignment
of structs are equivalent to memcpy.  So are long double assigns
btw - but the bugs that I think are important to fix are those
where even the source language semantics dictate they are well-defined.

I do think the long double issue eventually needs to be sorted out
in the middle-end.  So do I think SRA needs to be fixed to preserve
memcpy behavior of aggregate copies.

[Bug target/94103] Wrong optimization: reading value of a variable changes its representation for optimizer

[cvs-commit at gcc dot gnu.org]

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

--- Comment #17 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Richard Biener
<rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:4b0b6303dde0c32d936926de45b54cfe508fa677

commit r9-8444-g4b0b6303dde0c32d936926de45b54cfe508fa677
Author: Richard Biener <rguenther@suse.de>
Date:   Thu Mar 12 14:18:35 2020 +0100

    tree-optimization/94103 avoid CSE of loads with padding

    VN currently replaces a load of a 16 byte entity 128 bits of precision
    (TImode) with the result of a load of a 16 byte entity with 80 bits of
    mode precision (XFmode).  That will go downhill since if the padding
    bits are not actually filled with memory contents those bits are
    missing.

    2020-03-12  Richard Biener  <rguenther@suse.de>

            PR tree-optimization/94103
            * tree-ssa-sccvn.c (visit_reference_op_load): Avoid type
            punning when the mode precision is not sufficient.

            * gcc.target/i386/pr94103.c: New testcase.

[Bug target/94103] Wrong optimization: reading value of a variable changes its representation for optimizer

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #18 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.