[Bug tree-optimization/97960] New: [8/9/10/11 Regression] Wrong code at -O3 since r8-6511-g3ae129323d

[Bug tree-optimization/97960] New: [8/9/10/11 Regression] Wrong code at -O3 since r8-6511-g3ae129323d

[acoplan at gcc dot gnu.org]

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

            Bug ID: 97960
           Summary: [8/9/10/11 Regression] Wrong code at -O3 since
                    r8-6511-g3ae129323d
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: acoplan at gcc dot gnu.org
  Target Milestone: ---

GCC miscompiles the following C++ testcase at -O3 (reproduced on both x86 and
aarch64):

const int &c(const int &d, const int &f) {
  if (d < f)
    return f;
  return d;
}

short a[575];
unsigned b[25];
unsigned char g;
int main()
{
  for (int e = 0; e < 23; ++e)
    a[e * 23] = 16137;
  for (signed char h = c(g, 253) + 3; h < 24; h++)
    b[h] = 1064739102;
  for (int e = 0; e < 23; ++e)
    if (a[e * 23] != 16137)
      __builtin_abort();
}

When compiled with -O{0,g,s,1,2} (with or without -ftree-vectorize), the
program exits cleanly, but aborts when compiled with -O3 since the above
revision. The program also exits cleanly when compiled with clang on x86 and
aarch64 at various optimization levels.

[Bug tree-optimization/97960] [8/9/10/11 Regression] Wrong code at -O3 since r8-6511-g3ae129323d

[acoplan at gcc dot gnu.org]

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

--- Comment #1 from Alex Coplan <acoplan at gcc dot gnu.org> ---
C testcase:

const int *c(const int *p, const int *q)
{
  if (*p < *q)
    return q;
  return p;
}

short a[575];
unsigned b[25];
unsigned char g;
int main()
{
  for (int e = 0; e < 23; ++e)
    a[e * 23] = 16137;

  int t1 = g;
  int t2 = 253;
  signed char h = *c(&t1, &t2) + 3;

  for (; h < 24; h++)
    b[h] = 1064739102;

  for (int e = 0; e < 23; ++e)
    if (a[e * 23] != 16137)
      __builtin_abort();
}

[Bug tree-optimization/97960] [8/9/10/11 Regression] Wrong code at -O3 since r8-6511-g3ae129323d

[marxin at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
                 CC|                            |marxin at gcc dot gnu.org,
                   |                            |rsandifo at gcc dot gnu.org
   Last reconfirmed|                            |2020-11-23
           Priority|P3                          |P1
     Ever confirmed|0                           |1
   Target Milestone|---                         |8.5
           Keywords|                            |wrong-code

--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
Confirmed!

[Bug tree-optimization/97960] [8/9/10/11 Regression] Wrong code at -O3 since r8-6511-g3ae129323d

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P1                          |P2
                 CC|                            |rguenth at gcc dot gnu.org

[Bug tree-optimization/97960] [8/9/10/11 Regression] Wrong code at -O3 since r8-6511-g3ae129323d

[rguenth at gcc dot gnu.org]

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

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
Creating dr for b[_7]
        base_address: &b
        offset from base address: (ssizetype) ((sizetype) (signed char) _5 * 4)
        constant offset from base address: -1012
        step: 4
        base alignment: 32
        base misalignment: 0
        offset alignment: 4
        step alignment: 4
        base_object: b
        Access function 0: {(int) h_20, +, 1}_2

looks like the wrong sign for the constant offset.

#0  split_constant_offset_1 (type=<integer_type 0x7ffff68252a0 signed char>, 
    op0=<ssa_name 0x7ffff6816f78 6>, code=NOP_EXPR, op1=<tree 0x0>, 
    var=0x7fffffffba10, off=0x7fffffffba08, cache=..., limit=0x7fffffffc19c)

now, var_min/max is UNSIGNED -3 / -1 (precision 8), woff is 3
we compute -3 + 3 == 0 and overflow to true (UNSIGNED arithmetic)

_5 = (unsigned char) _35;
_6 = _5 + 3;
h_20 = (signed char) _6;

but then we continue with

                /* Calculate (ssizetype) OP0 - (ssizetype) TMP_VAR.  */
                widest_int diff = (widest_int::from (op0_min, sgn)
                                   - widest_int::from (var_min, sgn));

getting -253.  I remember this place has changed quite some times and
wide (sign-extended) vs. widest (signed) ints do not make it easier
to see what's correct ...

I'm defering to Richard here.  The C testcase trips at this point just
twice (ldist and vectorizer) so it's easy enough to 'catch' in a debugger.

[Bug tree-optimization/97960] [8/9/10/11 Regression] Wrong code at -O3 since r8-6511-g3ae129323d

[rsandifo at gcc dot gnu.org]

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

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |rsandifo at gcc dot gnu.org

--- Comment #4 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Mine.  Might not be able to get to it for a few days though.

[Bug tree-optimization/97960] [8/9/10/11 Regression] Wrong code at -O3 since r8-6511-g3ae129323d

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Richard, did you have time to look into this yet?

[Bug tree-optimization/97960] [8/9/10/11 Regression] Wrong code at -O3 since r8-6511-g3ae129323d

[rguenth at gcc dot gnu.org]

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

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
Is it maybe fixed with the latest re-org of split_constant_offset?

[Bug tree-optimization/97960] [8/9/10/11 Regression] Wrong code at -O3 since r8-6511-g3ae129323d

[acoplan at gcc dot gnu.org]

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

--- Comment #7 from Alex Coplan <acoplan at gcc dot gnu.org> ---
FWIW, the testcase now exits cleanly on AArch64 with current trunk.

[Bug tree-optimization/97960] [8/9/10/11 Regression] Wrong code at -O3 since r8-6511-g3ae129323d

[jakub at gcc dot gnu.org]

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Indeed, r11-5904-g4cf70c20cb10acd6fb1016611d05540728176b60 fixed it.  I'll add
the testcase into the testsuite after testing it.

[Bug tree-optimization/97960] [8/9/10/11 Regression] Wrong code at -O3 since r8-6511-g3ae129323d

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:eedda4e160856b7ac0c533ac9445161d0fd88660

commit r11-7033-geedda4e160856b7ac0c533ac9445161d0fd88660
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Feb 2 10:01:40 2021 +0100

    testsuite: Add testcase for already fixed PR [PR97960]

    This testcase has been fixed by
    r11-5904-g4cf70c20cb10acd6fb1016611d05540728176b60
    so I'm checking it in so that we can close the PR.

    2021-02-02  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/97960
            * g++.dg/torture/pr97960.C: New test.

[Bug tree-optimization/97960] [8/9/10 Regression] Wrong code at -O3 since r8-6511-g3ae129323d

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[8/9/10/11 Regression]      |[8/9/10 Regression] Wrong
                   |Wrong code at -O3 since     |code at -O3 since
                   |r8-6511-g3ae129323d         |r8-6511-g3ae129323d

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed on the trunk by

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

commit r11-5904-g4cf70c20cb10acd6fb1016611d05540728176b60                       
Author: Richard Sandiford <richard.sandiford@arm.com>                           
Date:   Thu Dec 10 12:10:00 2020 +0000                                          

    data-ref: Rework integer handling in split_constant_offset [PR98069]        

    PR98069 is about a case in which split_constant_offset miscategorises       
    an expression of the form:                                                  

      int foo;                                                                  
      …                                                                         
      POINTER_PLUS_EXPR<base, (sizetype)(INT_MIN - foo) * size>                 

    as:                                                                         

      base: base                                                                
      offset: (sizetype) (-foo) * size                                          
      init: INT_MIN * size                                                      

    “-foo” overflows when “foo” is INT_MIN, whereas the original expression     
    didn't overflow in that case.                                               

    As discussed in the PR trail, we could simply ignore the fact that          
    int overflow is undefined and treat it as a wrapping type, but that         
    is likely to pessimise quite a few cases.                                   

    This patch instead reworks split_constant_offset so that:                   

    - it treats integer operations as having an implicit cast to sizetype       
    - for integer operations, the returned VAR has type sizetype                

    In other words, the problem becomes to express:                             

      (sizetype) (OP0 CODE OP1)                                                 

    as:                                                                         

      VAR:sizetype + (sizetype) OFF:ssizetype                                   

    The top-level integer split_constant_offset will (usually) be a sizetype    
    POINTER_PLUS operand, so the extra cast to sizetype disappears.  But adding 
    the cast allows the conversion handling to defer a lot of the difficult     
    cases to the recursive split_constant_offset call, which can detect         
    overflow on individual operations.                                          

    The net effect is to analyse the access above as:                           

      base: base                                                                
      offset: -(sizetype) foo * size                                            
      init: INT_MIN * size                                                      

    See the comments in the patch for more details.                             

    gcc/                                                                        
            PR tree-optimization/98069                                          
            * tree-data-ref.c (compute_distributive_range): New function.       
            (nop_conversion_for_offset_p): Likewise.                            
            (split_constant_offset): In the internal overload, treat integer    
            expressions as having an implicit cast to sizetype and express      
            them accordingly.  Pass back the range of the original (uncast)     
            expression in a new range parameter.                                
            (split_constant_offset_1): Likewise.  Rework the handling of        
            conversions to account for the implicit sizetype casts.

[Bug tree-optimization/97960] [8/9/10 Regression] Wrong code at -O3 since r8-6511-g3ae129323d

[rsandifo at gcc dot gnu.org]

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

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|8.5                         |10.4

--- Comment #11 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
As discussed on irc, the fix was quite invasive, so it seems a bit
dangerous to backport further than GCC 10.  Will backport to GCC 10 in
the GCC 11.2 timeframe, once we've had more chance to see if there's
any fallout.

[Bug tree-optimization/97960] [8/9/10 Regression] Wrong code at -O3 since r8-6511-g3ae129323d

[rsandifo at gcc dot gnu.org]

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

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

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

--- Comment #12 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Tracking backports in PR98069

*** This bug has been marked as a duplicate of bug 98069 ***