On Thu, Dec 25, 2025 at 1:49 AM Richard Biener
<[email protected]> wrote:
>
>
>
> > Am 25.12.2025 um 06:46 schrieb Andrew Pinski 
> > <[email protected]>:
> >
> > Looking into the history here, we used to use the original pointer
> > type for the IVs. That was changed with r0-67159-gd482f417e4445f
> > where then we started to use ptr_type_node. And then that changed
> > again with r0-76106-g2052721560b232.
> > BUT this was all before POINTER_PLUS_EXPR was around 
> > (r0-81506-g5be014d5b728cf).
> > In the case of pointers even though it is undefined if they wrap,
> > the wrapping would have happened anyways.  Also the second
> > operand for pointer plus is treated as a signed integer
> > (even though it uses an unsigned integer type) so adding a value
> > that is negative for signed case but a big number of the
> > unsigned case is not undefined.
> >
> > So the reason why I am proposing this is to fix up a regression
> > after r16-5975-gfae0c6262 where we get and overlapping live
> > range for 2 variables which are the same:
> > ```
> >  # ivtmp.7_12 = PHI <ivtmp.7_5(5), ivtmp.7_15(3)>
> >  _3 = (int *) ivtmp.7_12;
> >  _4 = MEM[(int *)_3];
> >  if (_4 == val_10(D))
> >    goto <bb 6>; [5.50%]
> >  else
> >    goto <bb 5>; [94.50%]
> >  <bb 5> [local count: 958878295]:
> >  ivtmp.7_5 = ivtmp.7_12 + 4;
> >  if (ivtmp.7_5 != _20)
> >    goto <bb 4>; [94.50%]
> >  else
> >    goto <bb 7>; [5.50%]
> >  <bb 7> [local count: 59055800]:
> >  goto <bb 6>; [100.00%]
> >  <bb 6> [local count: 114863531]:
> >  # _6 = PHI <_3(4), 0B(7)>
> > ```
> > Sometimes cse2 fixes this up and other times it does not.  I have
> > not figured out what conditions that cse2 will fix it up or not.
> > I also noticed sometimes doing the removal of the forwarder causes
> > cse2 in other cases not to resolve this. So I felt it is better
> > to try to fix this up at the source, ivopts.
> >
> > Can anyone think of a reason why this might be wrong. I only think
> > if the pointer was used conditionally we might get an possible undefined
> > IV being selected.
>
> IVOPTS sometimes chooses to rewrite a ptr as another ptr plus the offset 
> between both.  So one reason for using unsigned was points-to info 
> correctness where POINTER_PLUS_EXPR has in-object constraints.  Conditional 
> vs. unconditional would indeed be another reason though a conditional 
> increment isn’t handled by SCEV.
>
> That said, some cases would be safe of course, but IIRC identifying all paths 
> that need different treatment is quite difficult.

Let me see about that. It is kind of interesting, I remember running
into the difference between pointers before in IVOPTs too.
Maybe the fix rtl cse is where the fixup should still happen after all.
I am going to look more into rtl cse.

Thanks,
Andrew

>
> Richard
>
>
> >  RFH: what is the best way to remove/change that
> > candidate when the pointer is used conditional.
> >
> > Bootstrapped and tested on x86_64-linux-gnu with just 4 minor
> > testcases changes needed.
> >
> >    PR tree-optimization/123070
> > gcc/ChangeLog:
> >
> >    * tree-ssa-loop-ivopts.cc (generic_type_for): Use ptr_type_node
> >    for pointer types.
> >
> > gcc/testsuite/ChangeLog:
> >
> >    * gcc.dg/tree-ssa/ivopts-5.c: Update dump scan.
> >    * gcc.dg/tree-ssa/ivopts-6.c: Likewise.
> >    * gcc.dg/tree-ssa/ivopts-8.c: Likewise.
> >    * gcc.dg/tree-ssa/ivopts-9.c: Likewise.
> >
> > Signed-off-by: Andrew Pinski <[email protected]>
> > ---
> > gcc/testsuite/gcc.dg/tree-ssa/ivopts-5.c | 2 +-
> > gcc/testsuite/gcc.dg/tree-ssa/ivopts-6.c | 2 +-
> > gcc/testsuite/gcc.dg/tree-ssa/ivopts-8.c | 2 +-
> > gcc/testsuite/gcc.dg/tree-ssa/ivopts-9.c | 2 +-
> > gcc/tree-ssa-loop-ivopts.cc              | 2 +-
> > 5 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ivopts-5.c 
> > b/gcc/testsuite/gcc.dg/tree-ssa/ivopts-5.c
> > index 7b9615f07f3..2b061879fb7 100644
> > --- a/gcc/testsuite/gcc.dg/tree-ssa/ivopts-5.c
> > +++ b/gcc/testsuite/gcc.dg/tree-ssa/ivopts-5.c
> > @@ -12,4 +12,4 @@ foo (int* mem, int sz, int val)
> >   return 0;
> > }
> >
> > -/* { dg-final { scan-tree-dump "inv_expr \[0-9\]: \\t\\(unsigned long\\) 
> > sz_\[0-9\]\\(D\\) \\* 4 \\+ \\(unsigned long\\) mem_\[0-9\]\\(D\\)" 
> > "ivopts" } } */
> > +/* { dg-final { scan-tree-dump "inv_expr \[0-9\]: \\tmem_\[0-9\]\\(D\\) 
> > \\+ \\(sizetype\\) sz_\[0-9\]\\(D\\) \\* 4" "ivopts" } } */
> > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ivopts-6.c 
> > b/gcc/testsuite/gcc.dg/tree-ssa/ivopts-6.c
> > index 08304293140..7d4f339e60d 100644
> > --- a/gcc/testsuite/gcc.dg/tree-ssa/ivopts-6.c
> > +++ b/gcc/testsuite/gcc.dg/tree-ssa/ivopts-6.c
> > @@ -12,4 +12,4 @@ foo (int* mem, int sz, int val)
> >   return 0;
> > }
> >
> > -/* { dg-final { scan-tree-dump "inv_expr \[0-9\]: \\t\\(unsigned long\\) 
> > sz_\[0-9\]\\(D\\) \\* 4 \\+ \\(unsigned long\\) mem_\[0-9\]\\(D\\)" 
> > "ivopts" } } */
> > +/* { dg-final { scan-tree-dump "inv_expr \[0-9\]: \\tmem_\[0-9\]\\(D\\) 
> > \\+ \\(sizetype\\) sz_\[0-9\]\\(D\\) \\* 4" "ivopts" } } */
> > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ivopts-8.c 
> > b/gcc/testsuite/gcc.dg/tree-ssa/ivopts-8.c
> > index a7fd3c9de37..05b8d7550c1 100644
> > --- a/gcc/testsuite/gcc.dg/tree-ssa/ivopts-8.c
> > +++ b/gcc/testsuite/gcc.dg/tree-ssa/ivopts-8.c
> > @@ -12,4 +12,4 @@ foo (int* mem, char sz, int val)
> >   return 0;
> > }
> >
> > -/* { dg-final { scan-tree-dump "inv_expr \[0-9\]: \\t\\(unsigned 
> > (long|int)\\) sz_\[0-9\]*\\(D\\) \\* 4 \\+ \\(unsigned (long|int)\\) 
> > mem_\[0-9\]*\\(D\\)" "ivopts" } } */
> > +/* { dg-final { scan-tree-dump "inv_expr \[0-9\]: \\tmem_\[0-9\]*\\(D\\) 
> > \\+ \\(sizetype\\) sz_\[0-9\]*\\(D\\) \\* 4" "ivopts" } } */
> > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ivopts-9.c 
> > b/gcc/testsuite/gcc.dg/tree-ssa/ivopts-9.c
> > index fb9656b88d7..092413d8039 100644
> > --- a/gcc/testsuite/gcc.dg/tree-ssa/ivopts-9.c
> > +++ b/gcc/testsuite/gcc.dg/tree-ssa/ivopts-9.c
> > @@ -12,4 +12,4 @@ foo (int* mem, unsigned char sz, int val)
> >   return 0;
> > }
> >
> > -/* { dg-final { scan-tree-dump "inv_expr \[0-9\]: \\t\\(unsigned 
> > (long|int)\\) sz_\[0-9\]\\(D\\) \\* 4 \\+ \\(unsigned (long|int)\\) 
> > mem_\[0-9\]\\(D\\)" "ivopts" } } */
> > +/* { dg-final { scan-tree-dump "inv_expr \[0-9\]: \\tmem_\[0-9\]*\\(D\\) 
> > \\+ \\(sizetype\\) sz_\[0-9\]*\\(D\\) \\* 4" "ivopts" } } */
> > diff --git a/gcc/tree-ssa-loop-ivopts.cc b/gcc/tree-ssa-loop-ivopts.cc
> > index ba727adc808..deaa68de200 100644
> > --- a/gcc/tree-ssa-loop-ivopts.cc
> > +++ b/gcc/tree-ssa-loop-ivopts.cc
> > @@ -2927,7 +2927,7 @@ static tree
> > generic_type_for (tree type)
> > {
> >   if (POINTER_TYPE_P (type))
> > -    return unsigned_type_for (type);
> > +    return ptr_type_node;
> >
> >   if (TYPE_UNSIGNED (type))
> >     return type;
> > --
> > 2.43.0
> >

Reply via email to