[Bug target/115936] New: [15 Regression] GCN vs. ivopts: replace constant_multiple_of with aff_combination_constant_multiple_p [PR114932]

[Bug target/115936] New: [15 Regression] GCN vs. ivopts: replace constant_multiple_of with aff_combination_constant_multiple_p [PR114932]

[tschwinge at gcc dot gnu.org]

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

            Bug ID: 115936
           Summary: [15 Regression] GCN vs. ivopts: replace
                    constant_multiple_of with
                    aff_combination_constant_multiple_p [PR114932]
           Product: gcc
           Version: 15.0
            Status: UNCONFIRMED
          Keywords: ice-checking, ice-on-valid-code, testsuite-fail
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: tschwinge at gcc dot gnu.org
                CC: ams at gcc dot gnu.org
  Target Milestone: ---
            Target: GCN

Recent commit r15-1809-g735edbf1e2479fa2323a2b4a9714fae1a0925f74 "ivopts:
replace constant_multiple_of with aff_combination_constant_multiple_p
[PR114932]" is causing one regression for '--target=amdgcn-amdhsa' (tested
'-march=gfx908', '-march=gfx1100'):

    @@ -98531,8 +98547,9 @@ PASS: gcc.dg/torture/pr101173.c   -O0  (test for
excess errors)
    PASS: gcc.dg/torture/pr101173.c   -O0  execution test
    PASS: gcc.dg/torture/pr101173.c   -O1  (test for excess errors)
    PASS: gcc.dg/torture/pr101173.c   -O1  execution test
    {+FAIL: gcc.dg/torture/pr101173.c   -O2  (internal compiler error:
verify_gimple failed)+}
    [-PASS:-]{+FAIL:+} gcc.dg/torture/pr101173.c   -O2  (test for excess
errors)
    [-PASS:-]{+UNRESOLVED:+} gcc.dg/torture/pr101173.c   -O2  [-execution
test-]{+compilation failed to produce executable+}
    PASS: gcc.dg/torture/pr101173.c   -O3 -fomit-frame-pointer -funroll-loops
-fpeel-loops -ftracer -finline-functions  (test for excess errors)
    PASS: gcc.dg/torture/pr101173.c   -O3 -fomit-frame-pointer -funroll-loops
-fpeel-loops -ftracer -finline-functions  execution test
    PASS: gcc.dg/torture/pr101173.c   -O3 -g  (test for excess errors)

    [...]/source-gcc/gcc/testsuite/gcc.dg/torture/pr101173.c: In function
'main':
    [...]/source-gcc/gcc/testsuite/gcc.dg/torture/pr101173.c:5:5: error:
invalid (pointer) operands 'plus_expr'
    ivtmp.39_65 = ivtmp.39_59 + 0B;
    during GIMPLE pass: ivopts
    [...]/source-gcc/gcc/testsuite/gcc.dg/torture/pr101173.c:5:5: internal
compiler error: verify_gimple failed
    0x20dcb22 internal_error(char const*, ...)
            [...]/source-gcc/gcc/diagnostic-global-context.cc:491
    0x11fe23e verify_gimple_in_cfg(function*, bool, bool)
            [...]/source-gcc/gcc/tree-cfg.cc:5678
    0x1092710 execute_function_todo
            [...]/source-gcc/gcc/passes.cc:2089
    0x1092c5b execute_todo
            [...]/source-gcc/gcc/passes.cc:2143

[Bug target/115936] [15 Regression] GCN vs. ivopts: replace constant_multiple_of with aff_combination_constant_multiple_p [PR114932]

[tnfchris at gcc dot gnu.org]

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

Tamar Christina <tnfchris at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
                 CC|                            |tnfchris at gcc dot gnu.org
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2024-07-15
           Assignee|unassigned at gcc dot gnu.org      |tnfchris at gcc dot gnu.org

--- Comment #1 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
odd thing to iCE on, mine.

[Bug target/115936] [15 Regression] GCN vs. ivopts: replace constant_multiple_of with aff_combination_constant_multiple_p [PR114932]

[tnfchris at gcc dot gnu.org]

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

Tamar Christina <tnfchris at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |15.0

--- Comment #2 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
Looks like IVopts has generated an invalid gimple:

ivtmp.39_65 = ivtmp.39_59 + 0B;

where the IVs are DI mode and the offset is a pointer.
This comes from this weird candidate:

Candidate 8:
  Var befor: ivtmp.39_59
  Var after: ivtmp.39_65
  Incr POS: before exit test
  IV struct:
    Type:       sizetype
    Base:       0
    Step:       0B
    Biv:        N
    Overflowness wrto loop niter:       No-overflow

Looks like this invalid candidate was always generated, but was not selected
before as the old constant_multiple_of bailed out due to the operand_equal_p
constraining the type of the arguments.

Question is why this invalid candidate was generated at all, and that's due to:

  /* Record common candidate with initial value zero.  */
  basetype = TREE_TYPE (iv->base);
  if (POINTER_TYPE_P (basetype))
    basetype = sizetype;
  record_common_cand (data, build_int_cst (basetype, 0), iv->step, use);

which for the case the type is a pointer changes the base but not the step.
this makes base + step no longer valid gimple.

So I believe fix is:

diff --git a/gcc/tree-ssa-loop-ivopts.cc b/gcc/tree-ssa-loop-ivopts.cc
index 5fc188ae3f8..d590d6a9b78 100644
--- a/gcc/tree-ssa-loop-ivopts.cc
+++ b/gcc/tree-ssa-loop-ivopts.cc
@@ -3547,7 +3547,8 @@ add_iv_candidate_for_use (struct ivopts_data *data,
struct iv_use *use)
   basetype = TREE_TYPE (iv->base);
   if (POINTER_TYPE_P (basetype))
     basetype = sizetype;
-  record_common_cand (data, build_int_cst (basetype, 0), iv->step, use);
+  record_common_cand (data, build_int_cst (basetype, 0),
+                     fold_convert (basetype, iv->step), use);


which fixes the ICE. Will regtest and submit.

[Bug target/115936] [15 Regression] GCN vs. ivopts: replace constant_multiple_of with aff_combination_constant_multiple_p [PR114932]

[rguenth at gcc dot gnu.org]

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

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
iv->step should never be a pointer type

[Bug target/115936] [15 Regression] GCN vs. ivopts: replace constant_multiple_of with aff_combination_constant_multiple_p [PR114932]

[tnfchris at gcc dot gnu.org]

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

--- Comment #4 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #3)
> iv->step should never be a pointer type

That's what I initially thought too.  My suspicion is that there is some code
that tries to create the 0 offset.

I'll try to track down where the IV is created.

0 + 0B is a weird candidate either way.

[Bug target/115936] [15 Regression] GCN vs. ivopts: replace constant_multiple_of with aff_combination_constant_multiple_p [PR114932]

[tnfchris at gcc dot gnu.org]

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

--- Comment #5 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #3)
> iv->step should never be a pointer type

This is created by SCEV.

simple_iv_with_niters in the case where no CHREC is found creates an IV with
base == ev, offset == 0;

however in this case EV is a POINTER_PLUS_EXPR and so the type is a pointer.
it ends up creating an unusable expression.

following the remaining code, it looks like this should be

diff --git a/gcc/tree-scalar-evolution.cc b/gcc/tree-scalar-evolution.cc
index 5aa95a2497a..abb2bad7773 100644
--- a/gcc/tree-scalar-evolution.cc
+++ b/gcc/tree-scalar-evolution.cc
@@ -3243,7 +3243,11 @@ simple_iv_with_niters (class loop *wrto_loop, class loop
*use_loop,
   if (tree_does_not_contain_chrecs (ev))
     {
       iv->base = ev;
-      iv->step = build_int_cst (TREE_TYPE (ev), 0);
+      tree ev_type = TREE_TYPE (ev);
+      if (POINTER_TYPE_P (ev_type))
+       ev_type = sizetype;
+
+      iv->step = build_int_cst (ev_type, 0);
       iv->no_overflow = true;
       return true;
     }

So I think there are two bugs here.  I also think the IVopts one is a bug, as
it's clearly changing the type and introducing a mismatch there too.

I'm regression testing both changes.

[Bug target/115936] [15 Regression] GCN vs. ivopts: replace constant_multiple_of with aff_combination_constant_multiple_p [PR114932]

[tnfchris at gcc dot gnu.org]

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

--- Comment #6 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #3)
> iv->step should never be a pointer type

This is created by SCEV.

simple_iv_with_niters in the case where no CHREC is found creates an IV with
base == ev, offset == 0;

however in this case EV is a POINTER_PLUS_EXPR and so the type is a pointer.
it ends up creating an unusable expression.

following the remaining code, it looks like this should be

diff --git a/gcc/tree-scalar-evolution.cc b/gcc/tree-scalar-evolution.cc
index 5aa95a2497a..abb2bad7773 100644
--- a/gcc/tree-scalar-evolution.cc
+++ b/gcc/tree-scalar-evolution.cc
@@ -3243,7 +3243,11 @@ simple_iv_with_niters (class loop *wrto_loop, class loop
*use_loop,
   if (tree_does_not_contain_chrecs (ev))
     {
       iv->base = ev;
-      iv->step = build_int_cst (TREE_TYPE (ev), 0);
+      tree ev_type = TREE_TYPE (ev);
+      if (POINTER_TYPE_P (ev_type))
+       ev_type = sizetype;
+
+      iv->step = build_int_cst (ev_type, 0);
       iv->no_overflow = true;
       return true;
     }

So I think there are two bugs here.  I also think the IVopts one is a bug, as
it's clearly changing the type and introducing a mismatch there too.

I'm regression testing both changes.

[Bug tree-optimization/115936] [15 Regression] GCN vs. ivopts: replace constant_multiple_of with aff_combination_constant_multiple_p [PR114932]

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tamar Christina <tnfchris@gcc.gnu.org>:

https://gcc.gnu.org/g:0135a90de5a99b51001b6152d8b548151ebfa1c3

commit r15-2099-g0135a90de5a99b51001b6152d8b548151ebfa1c3
Author: Tamar Christina <tamar.christina@arm.com>
Date:   Wed Jul 17 16:22:14 2024 +0100

    middle-end: fix 0 offset creation and folding [PR115936]

    As shown in PR115936 SCEV and IVOPTS create an invalidate IV when the IV is
    a pointer type:

    ivtmp.39_65 = ivtmp.39_59 + 0B;

    where the IVs are DI mode and the offset is a pointer.
    This comes from this weird candidate:

    Candidate 8:
      Var befor: ivtmp.39_59
      Var after: ivtmp.39_65
      Incr POS: before exit test
      IV struct:
        Type:       sizetype
        Base:       0
        Step:       0B
        Biv:        N
        Overflowness wrto loop niter:       No-overflow

    This IV was always created just ended up not being used.

    This is created by SCEV.

    simple_iv_with_niters in the case where no CHREC is found creates an IV
with
    base == ev, offset == 0;

    however in this case EV is a POINTER_PLUS_EXPR and so the type is a
pointer.
    it ends up creating an unusable expression.

    gcc/ChangeLog:

            PR tree-optimization/115936
            * tree-scalar-evolution.cc (simple_iv_with_niters): Use sizetype
for
            pointers.

[Bug tree-optimization/115936] [15 Regression] GCN vs. ivopts: replace constant_multiple_of with aff_combination_constant_multiple_p [PR114932]

[tnfchris at gcc dot gnu.org]

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

Tamar Christina <tnfchris at gcc dot gnu.org> changed:

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

--- Comment #8 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
Fixed, thanks for the report.  Bug is latent on branches so won't backport for
now.