Bug 104162 - [12 Regression] Missed CSE after lowering of &MEM[ptr_1 + CST]
Summary: [12 Regression] Missed CSE after lowering of &MEM[ptr_1 + CST]
Status: ASSIGNED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 12.0
: P2 normal
Target Milestone: 13.3
Assignee: Richard Biener
URL:
Keywords: missed-optimization
: 108074 (view as bug list)
Depends on:
Blocks:
 
Reported: 2022-01-21 13:01 UTC by Richard Biener
Modified: 2023-07-27 09:22 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work: 13.0
Known to fail:
Last reconfirmed: 2022-01-21 00:00:00


Attachments
untested patch (1.74 KB, patch)
2022-01-26 14:39 UTC, Richard Biener
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Biener 2022-01-21 13:01:20 UTC
In the testcase of PR99673 we can see

  <bb 2> [local count: 1073741824]:
  _13 = &MEM[(struct B *)pc_2(D) + 1B].i;
  .ASAN_CHECK (6, _13, 4, 4);
  _1 = MEM[(struct B *)pc_2(D) + 1B].i;
  _14 = &pd_4(D)->i;
  .ASAN_CHECK (7, _14, 4, 4);
  pd_4(D)->i = _1;
  _9 = (sizetype) i_6(D);
  _10 = _9 * 16;
  _11 = _10 + 4;
  _12 = pc_2(D) + 1;
  psa_7 = _12 + _11;
  f (psa_7);

and FRE5 is not CSEing _12 to _13.
Comment 1 Richard Biener 2022-01-21 13:01:39 UTC
I will have a look.
Comment 2 Richard Biener 2022-01-21 13:13:18 UTC
Testcase that did not regress:

struct S { int i; };

void foo (int *);

void bar (char *p)
{
  foo (&((struct S *)(p + 1))->i);
  foo ((int *)(p + 1));
}

Testcase that did regress (use -fgimple):

struct S { int i; };

void foo (int *);

void __GIMPLE (ssa)
bar (char * p)
{
  int * D_1997;
  int * _2;
  char * _3;

  __BB(2):
  _2 = &__MEM <struct S> ((struct S *)p_5(D) + _Literal (struct S *) 1).i;
  foo (_2);
  _3 = &__MEM <int> ((struct S *)p_5(D) + _Literal (struct S *)1);
  foo (_3);
  return;
}

it regressed because forwprop now rewrites one of the &MEMs:

--- t.c.033t.ccp1       2022-01-21 14:12:01.392883591 +0100
+++ t.c.034t.forwprop1  2022-01-21 14:12:01.392883591 +0100
@@ -10,7 +10,7 @@
   <bb 2> :
   _2_6 = &MEM[(struct S *)p_5(D) + 1B].i;
   foo (_2_6);
-  _3_9 = &MEM[(struct S *)p_5(D) + 1B];
+  _3_9 = p_5(D) + 1;
   foo (_3_9);
   return;
Comment 3 Andrew Pinski 2022-01-21 16:10:06 UTC
Right my original version of the lowering handled this. I guess we need to handle the case where we have a handled reference too. But that would regress PR 99673 again. I will take a look this weekend.
Comment 4 Richard Biener 2022-01-24 09:50:09 UTC
Btw, in VN it would be nice to handle

struct S { int i; };

int i;
int bar (char *p)
{
  char *q = p + 1;
  i = 1;
  char *r = (char *)&(((struct S *)&p[i])->i);
  return q == r;
}

the main issue here is that this is vn_reference vs. vn_nary handling and this
transitions from vn_reference to possibly vn_nary with valueization.  For VN
&MEM[p + 1] was more canonical and we could go back to this when seeing
pointer-plus -- at least when lookup & simplification does not produce a
redundancy.

Handling this in VN only and only when inlining is complete might also avoid
regressing the testcase again.
Comment 5 Richard Biener 2022-01-26 13:48:49 UTC
(In reply to Richard Biener from comment #4)
> Btw, in VN it would be nice to handle
> 
> struct S { int i; };
> 
> int i;
> int bar (char *p)
> {
>   char *q = p + 1;
>   i = 1;
>   char *r = (char *)&(((struct S *)&p[i])->i);
>   return q == r;
> }

Rather the following

struct S { int a[4]; };

int i;
int bar (struct S *p)
{
  char *q = (char *)p + 4;
  i = 1;
  int *r = &((struct S *)p)->a[i];
  return q == (char *)r;
}

> the main issue here is that this is vn_reference vs. vn_nary handling and
> this
> transitions from vn_reference to possibly vn_nary with valueization.  For VN
> &MEM[p + 1] was more canonical and we could go back to this when seeing
> pointer-plus -- at least when lookup & simplification does not produce a
> redundancy.
> 
> Handling this in VN only and only when inlining is complete might also avoid
> regressing the testcase again.

The adjusted testcase is folded in forwprop after inlining which handles
p+4 == &p->a[1]
Comment 6 Richard Biener 2022-01-26 14:39:26 UTC
Created attachment 52293 [details]
untested patch

Like this - it unfortunately regresses the diagnostic case in PR99673 again but should leave early __builtin_object_size untouched.
Comment 7 GCC Commits 2022-05-05 12:41:42 UTC
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

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

commit r13-137-gee1cb43bc76de800efa0ade687b0cd28e62a5f82
Author: Richard Biener <rguenther@suse.de>
Date:   Wed Jan 26 15:34:54 2022 +0100

    tree-optimization/104162 - CSE of &MEM[ptr].a[i] and ptr + CST
    
    This adds the capability to value-numbering of treating complex
    address expressions where the offset becomes invariant as equal
    to a POINTER_PLUS_EXPR.  This restores CSE that is now prevented
    by early lowering of &MEM[ptr + CST] to a POINTER_PLUS_EXPR.
    
    Unfortunately this regresses gcc.dg/asan/pr99673.c again, so
    the testcase is adjusted accordingly.
    
    2022-01-26  Richard Biener  <rguenther@suse.de>
    
            PR tree-optimization/104162
            * tree-ssa-sccvn.cc (vn_reference_lookup): Handle
            &MEM[_1 + 5].a[i] like a POINTER_PLUS_EXPR if the offset
            becomes invariant.
            (vn_reference_insert): Likewise.
    
            * gcc.dg/tree-ssa/ssa-fre-99.c: New testcase.
            * gcc.dg/asan/pr99673.c: Adjust.
Comment 8 Martin Liška 2022-12-12 13:32:12 UTC
*** Bug 108074 has been marked as a duplicate of this bug. ***
Comment 9 Richard Biener 2023-04-26 06:55:41 UTC
GCC 13.1 is being released, retargeting bugs to GCC 13.2.
Comment 10 Richard Biener 2023-07-27 09:22:37 UTC
GCC 13.2 is being released, retargeting bugs to GCC 13.3.