On Thu, Dec 25, 2025 at 3:41 AM Richard Biener <[email protected]> wrote: > > > > > Am 25.12.2025 um 11:25 schrieb Andrew Pinski > > <[email protected]>: > > > > 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. > > Note that since I removed the bogus base term handling from binary RTL ops > issues from this might be even harder to trigger. I’d expect the alias > oracle walking of SSA defs possibly prone to expose this as well as PTA if > you leave around pointer-plus. Just punning to integers for the plus > operation might be OK - not sure if that would resolve your original issue.
Yes, just punning to integers only for the addition should fix this issue too. Because we won't have any overlapping register live ranges happening. Or there is one, it was going to be there with or without the change of types for the addition. Let me see if there is a way to get the punting happening too. Thanks, Andrew > > Richard > > > 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 > >>>
