On Wed, Jul 23, 2025 at 9:08 AM Andrew Pinski <pins...@gmail.com> wrote:
>
> On Wed, Jul 23, 2025 at 12:03 AM Richard Biener
> <richard.guent...@gmail.com> wrote:
> >
> > On Tue, Jul 22, 2025 at 7:57 PM Andrew Pinski <quic_apin...@quicinc.com> 
> > wrote:
> > >
> > > The switch conversion code will generate an array with VLA vector 
> > > constants in it
> > > in some cases but this does not work as the length of the vector type is 
> > > not known
> > > at compile time; only the min size is known.
> > >
> > > I tried to reject this in initializer_constant_valid_p but code from the 
> > > C++ front-end
> > > will call initializer_constant_valid_p for `vector_arrayx4()` and then 
> > > will cause an ICE
> > > with g++.target/aarch64/sve/pr116595.C (and 
> > > g++.target/riscv/rvv/autovec/pr116595.C).
> > >
> > > Built and tested for aarch64-linux-gnu.
> > >
> > >         PR tree-optimization/121091
> > >
> > > gcc/ChangeLog:
> > >
> > >         * tree-switch-conversion.cc (switch_conversion::check_final_bb): 
> > > Reject vector types which
> > >         have a non-constant number of elements.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > >         * gcc.target/aarch64/sve/pr121091-1.c: New test.
> > >
> > > Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> > > ---
> > >  .../gcc.target/aarch64/sve/pr121091-1.c       | 25 +++++++++++++++++++
> > >  gcc/tree-switch-conversion.cc                 |  3 +++
> > >  2 files changed, 28 insertions(+)
> > >  create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr121091-1.c
> > >
> > > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr121091-1.c 
> > > b/gcc/testsuite/gcc.target/aarch64/sve/pr121091-1.c
> > > new file mode 100644
> > > index 00000000000..ea2e5ce6b6a
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr121091-1.c
> > > @@ -0,0 +1,25 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-O2" } */
> > > +
> > > +/* PR tree-optimization/121091 */
> > > +/* Switch conversion would convert this
> > > +   into static constant array but since
> > > +   svbool_t is a VLA type, it can't be
> > > +   stored in a constant pool. */
> > > +
> > > +#include "arm_sve.h"
> > > +
> > > +svbool_t e(int mode, svbool_t pg) {
> > > +    switch (mode) {
> > > +        case 0:
> > > +            pg = svptrue_pat_b16(SV_VL6);
> > > +            break;
> > > +        case 1:
> > > +            pg = svpfalse_b();
> > > +            break;
> > > +        case 2:
> > > +            pg = svptrue_pat_b16(SV_VL2);
> > > +            break;
> > > +    }
> > > +  return pg;
> > > +}
> > > diff --git a/gcc/tree-switch-conversion.cc b/gcc/tree-switch-conversion.cc
> > > index d0882879e61..dc9f67bd272 100644
> > > --- a/gcc/tree-switch-conversion.cc
> > > +++ b/gcc/tree-switch-conversion.cc
> > > @@ -636,6 +636,9 @@ switch_conversion::check_final_bb ()
> > >               val = gimple_phi_arg_def (phi, i);
> > >               if (!is_gimple_ip_invariant (val))
> > >                 reason = "non-invariant value from a case";
> > > +             else if (VECTOR_TYPE_P (TREE_TYPE (val))
> > > +                      && !TYPE_VECTOR_SUBPARTS (TREE_TYPE 
> > > (val)).is_constant ())
> > > +               reason = "VLA vector type";
> >
> > OK with me, but then I see ...
> >
> > >               else
> > >                 {
> > >                   reloc = initializer_constant_valid_p (val, TREE_TYPE 
> > > (val));
> >
> > ... this and wonder why initializer_constant_valid_p says a VLA vector
> > constant is valid?
>
> I tried to describe why changing initializer_constant_valid_p won't
> work in the commit message:
> > > I tried to reject this in initializer_constant_valid_p but code from the 
> > > C++ front-end
> > > will call initializer_constant_valid_p for `vector_arrayx4()` and then 
> > > will cause an ICE
> > > with g++.target/aarch64/sve/pr116595.C (and 
> > > g++.target/riscv/rvv/autovec/pr116595.C).
>
> Maybe I was not clear enough or there was not enough information there on it.

Nah, I was too lazy to read all of it ... :/

Still I think initializer_constant_valid_p should be fixed (and the
fallout, obviously).  The
patch is OK in the meantime - also for backports to branches.

Can you open a bugreport to track the initializer_constant_valid_p issue?

Richard.

> Thanks,
> Andrew Pinski
>
> >
> > Richard.
> >
> > > --
> > > 2.43.0
> > >

Reply via email to