[Bug tree-optimization/107920] New: [13 Regression] ICE in execute_todo, at passes.cc:2140

[Bug tree-optimization/107920] New: [13 Regression] ICE in execute_todo, at passes.cc:2140

[asolokha at gmx dot com]

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

            Bug ID: 107920
           Summary: [13 Regression] ICE in execute_todo, at passes.cc:2140
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: asolokha at gmx dot com
  Target Milestone: ---
            Target: aarch64-linux-gnu

gcc 13.0.0 20221127 snapshot (g:4ac789789ea866ced24613e4778725d8ec6b0ecf) ICEs
when compiling the following testcase, reduced from
gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr96463-2.c, w/
-march=armv8-a+sve2 -O1 -fno-tree-ccp -fno-tree-forwprop:

#include "arm_sve.h"

svint8_t
test_s8(int8_t *x)
{
  return svld1rq_s8 (svptrue_b8 (), &x[0]);
}

% aarch64-linux-gnu-gcc-13 -march=armv8-a+sve2 -O1 -fno-tree-ccp
-fno-tree-forwprop -c xgdgkm2d.c
during GIMPLE pass: fre
xgdgkm2d.c: In function 'test_s8':
xgdgkm2d.c:7:1: internal compiler error: in execute_todo, at passes.cc:2140
    7 | }
      | ^
0x751734 execute_todo
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-13.0.0_p20221127/work/gcc-13-20221127/gcc/passes.cc:2140

[Bug tree-optimization/107920] [13 Regression] ICE in execute_todo, at passes.cc:2140

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |13.0

[Bug tree-optimization/107920] [13 Regression] ICE in execute_todo, at passes.cc:2140

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-11-29

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed.
Before:
  _1 = { -1, ... };
  _4 = svld1rq_s8 (_1, x_3(D));

after fre:
 _5 = MEM <vector(16) signed char> [(signed char * {ref-all})x_3(D)];
  _4 = VEC_PERM_EXPR <_5, _5, { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13,
14, 15, ... }>;

[Bug tree-optimization/107920] [13 Regression] ICE in execute_todo, at passes.cc:2140

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
It is specifically svld1rq_s8 folding into:
  # VUSE <.MEM>
  _5 = MEM <vector(16) signed char> [(signed char * {ref-all})x_3(D)];


Which is done by svld1rq_impl::fold (aarch64-sve-builtins-base.cc:1214).

[Bug tree-optimization/107920] [13 Regression] ICE in execute_todo, at passes.cc:2140

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #2)
> It is specifically svld1rq_s8 folding into:
>   # VUSE <.MEM>
>   _5 = MEM <vector(16) signed char> [(signed char * {ref-all})x_3(D)];
> 
> 
> Which is done by svld1rq_impl::fold (aarch64-sve-builtins-base.cc:1214).

Which was added by r13-1055-g494bec025002df .

The question becomes is gimple folding allowed to add MEM references here or
not?

[Bug tree-optimization/107920] [13 Regression] ICE in execute_todo, at passes.cc:2140

[pinskia at gcc dot gnu.org]

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #3)
> Which was added by r13-1055-g494bec025002df .
> 
> The question becomes is gimple folding allowed to add MEM references here or
> not?

No other target (or even base builtins) has a gimple folder that is conditional
based on the input that adds a load as far as I see which is one of the reasons
why this was not hit before. I suspect anytime we invoke the gimple folder, we
might run into this issue .....

[Bug tree-optimization/107920] [13 Regression] ICE in execute_todo, at passes.cc:2140

[pinskia at gcc dot gnu.org]

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

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Note I tried with (same options):
void
f (void)
{
  int t = 16;
  int t1 = 4;
  int t2 = 8;
  int t3 = 0;
  __builtin_aarch64_im_lane_boundsi (t, t1, t3);
}

But since we are removing the vdef via unlink_stmt_vdef (stmt); and not
replacing it with a new load it works just fine.

I am now trying to remember we had this issue before but I can't remember the
solution ...

[Bug target/107920] [13 Regression] ICE in execute_todo, at passes.cc:2140

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=50389
          Component|tree-optimization           |target

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The solution was gsi_replace_with_seq_vops/gimplify_and_update_call_from_tree
in gimple-fold.cc .

Which case this is a target issue. The target gimple folder should be handling
this ...

[Bug target/107920] [13 Regression] ICE in execute_todo, at passes.cc:2140

[pinskia at gcc dot gnu.org]

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

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Something like this should fix the issue (note setting the location should be
done also):
diff --git a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
index 6347407555f..d0a0a3a0a46 100644
--- a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
+++ b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
@@ -1232,6 +1232,15 @@ public:
        tree mem_ref_op = fold_build2 (MEM_REF, access_type, arg1, zero);
        gimple *mem_ref_stmt
          = gimple_build_assign (mem_ref_lhs, mem_ref_op);
+
+       // Set the location of load
+       gimple_set_location (mem_ref_stmt, gimple_location (f.call));
+
+       // Set the vuse for the load from the call
+       tree reaching_vuse = gimple_vuse (f.call);
+       gcc_assert (reaching_vuse && TREE_CODE (reaching_vuse) == SSA_NAME);
+       gimple_set_vuse (mem_ref_stmt, reaching_vuse);
+
        gsi_insert_before (f.gsi, mem_ref_stmt, GSI_SAME_STMT);

        int source_nelts = TYPE_VECTOR_SUBPARTS (access_type).to_constant ();

[Bug target/107920] [13 Regression] ICE in execute_todo, at passes.cc:2140

[pinskia at gcc dot gnu.org]

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

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
There is another question, how does this folding interact with
-fnon-call-exceptions and is it still correct?

E.g. compile at -O1 -march=armv8-a+sve2 -fnon-call-exceptions:

#include "arm_sve.h"

svint8_t
test_s8(int8_t *x)
{
  try {
  return svld1rq_s8 (svptrue_b8 (), &x[0]);
  } catch(...)
  {
      return (svint8_t){};
  }
}
---- CUT ----
During ccp1, the catch part is removed ....

[Bug target/107920] [13 Regression] ICE in execute_todo, at passes.cc:2140

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #3)
> (In reply to Andrew Pinski from comment #2)
> > It is specifically svld1rq_s8 folding into:
> >   # VUSE <.MEM>
> >   _5 = MEM <vector(16) signed char> [(signed char * {ref-all})x_3(D)];
> > 
> > 
> > Which is done by svld1rq_impl::fold (aarch64-sve-builtins-base.cc:1214).
> 
> Which was added by r13-1055-g494bec025002df .
> 
> The question becomes is gimple folding allowed to add MEM references here or
> not?

Yes, but the folding needs to properly handle virtual operands.  Instead it
does

-  # VUSE <.MEM_2(D)>
-  _4 = svld1rq_s8 (_1, x_3(D));
+  # VUSE <.MEM>
+  _5 = MEM <vector(16) signed char> [(signed char * {ref-all})x_3(D)];
+  _4 = VEC_PERM_EXPR <_5, _5, { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13,
14, 15, ... }>;

see how gimple-fold.cc handles them.

[Bug target/107920] [13 Regression] ICE in execute_todo, at passes.cc:2140

[rguenth at gcc dot gnu.org]

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

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #7)
> Something like this should fix the issue (note setting the location should
> be done also):
> diff --git a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> index 6347407555f..d0a0a3a0a46 100644
> --- a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> +++ b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> @@ -1232,6 +1232,15 @@ public:
>         tree mem_ref_op = fold_build2 (MEM_REF, access_type, arg1, zero);
>         gimple *mem_ref_stmt
>           = gimple_build_assign (mem_ref_lhs, mem_ref_op);
> +
> +       // Set the location of load
> +       gimple_set_location (mem_ref_stmt, gimple_location (f.call));
> +
> +       // Set the vuse for the load from the call
> +       tree reaching_vuse = gimple_vuse (f.call);
> +       gcc_assert (reaching_vuse && TREE_CODE (reaching_vuse) == SSA_NAME);
> +       gimple_set_vuse (mem_ref_stmt, reaching_vuse);
> +
>         gsi_insert_before (f.gsi, mem_ref_stmt, GSI_SAME_STMT);
> 
>         int source_nelts = TYPE_VECTOR_SUBPARTS (access_type).to_constant ();

All this should build a sequence of the replacement stmts and then use
gsi_replace_with_seq.

Note transfering EH will not work during folding since that would require
splitting the block.  Does the builtin even throw with non-call EH?  Why
does it throw without?

[Bug target/107920] [13 Regression] ICE in execute_todo, at passes.cc:2140

[prathamesh3492 at gcc dot gnu.org]

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

--- Comment #11 from prathamesh3492 at gcc dot gnu.org ---
Created attachment 53992
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53992&action=edit
untested fix

Thanks for the suggestions. The attached patch uses gsi_replace_with_seq_vops
for preserving VUSE, which prevents the issue and results in following dump for
fre:

  <bb 2> :
  # VUSE <.MEM_2(D)>
  _5 = MEM <vector(16) signed char> [(signed char * {ref-all})x_3(D)];
  _4 = VEC_PERM_EXPR <_5, _5, { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13,
14, 15, ... }>;
  # VUSE <.MEM_2(D)>
  return _4;

and following code-gen:
test_s8:
        ldr     q0, [x0]
        dup     z0.q, z0.q[0]
        ret

I am not sure tho if using update_ssa in the patch is ideal. If not, could you
please suggest a better alternative ?

Thanks,
Prathamesh

[Bug target/107920] [13 Regression] ICE in execute_todo, at passes.cc:2140

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The update_ssa call is very wrong, you just can't do that for every gimple
backend builtin call you fold, you could have millions of them and update_ssa
is a global whole function expensive function.
Either one doesn't ensure vops are properly updated and queues a
TODO_update_ssa_only_virtuals or so at the end of a pass, or one makes sure to
update it properly.  For the gimple folder, I think only the second option is
reasonable...

[Bug target/107920] [13 Regression] ICE in execute_todo, at passes.cc:2140

[prathamesh3492 at gcc dot gnu.org]

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

prathamesh3492 at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #53992|0                           |1
        is obsolete|                            |

--- Comment #13 from prathamesh3492 at gcc dot gnu.org ---
Created attachment 54001
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54001&action=edit
untested fix 2

Hi Jakub, thanks for the suggestions. The issue with previous patch was it used
gimple_seq_add_stmt, and passed resulting seq to gsi_replace_with_seq_vops. The
attached patch uses gimple_seq_add_stmt_without_update instead, which works to
resolve the issue without calling update_ssa(). Does it look OK ?

Thanks,
Prathamesh

[Bug target/107920] [13 Regression] ICE in execute_todo, at passes.cc:2140

[prathamesh3492 at gcc dot gnu.org]

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

--- Comment #14 from prathamesh3492 at gcc dot gnu.org ---
Posted patch:
https://gcc.gnu.org/pipermail/gcc-patches/2022-December/607714.html

Thanks,
Prathamesh

[Bug target/107920] [13 Regression] ICE in execute_todo, at passes.cc:2140

[cvs-commit at gcc dot gnu.org]

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

--- Comment #15 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Prathamesh Kulkarni
<prathamesh3492@gcc.gnu.org>:

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

commit r13-4533-gcb6922f490d3133e4ccbc1190b555d16695fc9c3
Author: Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org>
Date:   Wed Dec 7 13:34:02 2022 +0530

    PR107920: Fix handling of virtual operands and disable folding for
-fnon-call-exceptions.

    gcc/ChangeLog:
            PR target/107920
            * config/aarch64/aarch64-sve-builtins-base.cc: Use
            gsi_replace_with_seq_vops to handle virtual operands, and gate
            the transform on !flag_non_call_exceptions.
            * gimple-fold.cc (gsi_replace_with_seq_vops): Make function non
static.
            * gimple-fold.h (gsi_replace_with_seq_vops): Declare.

    gcc/testsuite/ChangeLog:
            PR target/107920
            * gcc.target/aarch64/sve/acle/general/pr107920.c: New test.
            * g++.target/aarch64/sve/pr107920.C: Likewise.

[Bug target/107920] [13 Regression] ICE in execute_todo, at passes.cc:2140

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #16 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Assuming this is now fixed.