On Thu, 2025-11-06 at 09:40 +0100, Richard Biener wrote:
> On Thu, 6 Nov 2025, Avinash Jayakar wrote:
>
> > Hi,
> >
> > Based on the approach mentioned by Andrew, below is a patch to fix
> > PR122126.
> > Bootstrapped and regtested on powerpc64le and x86 with no
> > regressions. Kindly
> > review.
> >
> > Thanks and regards,
> > Avinash Jayakar
> >
> > The function gimple_expand_vec_set_expr in the isel pass, converted
> > VIEW_CONVERT_EXPR to VEC_SET_EXPR without checking the bounds on
> > the index,
> > which cause ICE on targets that supported VEC_SET_EXPR like x86 and
> > powerpc.
> > This patch adds a bound check on the index operand and rejects the
> > conversion
> > if index is out of bound.
>
> I think this is somewhat fragile since if we ever get sth like
>
> _1 = 1345677;
> _2 = ... the access ...
>
> the variable bound will not be checked but TER might pull in the
> constant
> definition. I think targets should not ICE on UB here?
>
Thanks for the quick review. So to reproduce the scenario you mentioned
following is a simple test case
#define vect16 __attribute__((vector_size(16)))
void func_54() {
volatile vect16 unsigned BS_VAR_0;
volatile idx = 12;
BS_VAR_0[idx] = 4;
}
Im not so familiar with TER, is it a combination of passes like the
constant propagation?
I looked at the powerpc target to fix this issue. So there in
rs6000_expand_vector_set function, it uses gen_vsx_set_v4si_p9 to
generate the vec_set insn without checking the index. So during the
next pass, since there is a constraint on the operand in define_insn
saying const_0_to_3_operand, it fails to recognize the insn that was
generated in the expand pass.
So a check can be inserted to see the value of constant in
rs6000_expand_vector_set function, but I am not sure what has to be
generated since this is UB. Any suggestions here?
Thanks and regards,
Avinash Jayakar
> I'm also missing a testcase here.
>
> Thanks,
> Richard.
>
> > 2025-11-06 Avinash Jayakar <[email protected]>
> >
> > gcc/ChangeLog:
> > PR tree-optimization/122126
> > * gimple-isel.cc (gimple_expand_vec_set_extract_expr): Add
> > bound check.
> > ---
> > gcc/gimple-isel.cc | 14 ++++++++++++++
> > 1 file changed, 14 insertions(+)
> >
> > diff --git a/gcc/gimple-isel.cc b/gcc/gimple-isel.cc
> > index b5dc579ff46..2593cae1aad 100644
> > --- a/gcc/gimple-isel.cc
> > +++ b/gcc/gimple-isel.cc
> > @@ -102,6 +102,20 @@ gimple_expand_vec_set_extract_expr (struct
> > function *fun,
> > tree pos = TREE_OPERAND (ref, 1);
> >
> > tree view_op0 = TREE_OPERAND (op0, 0);
> > +
> > + tree idx = TREE_OPERAND (ref, 1);
> > + // if index is a constant, then check the bounds
> > + if (TREE_CODE (idx) == INTEGER_CST)
> > + {
> > + poly_uint64 nelts = TYPE_VECTOR_SUBPARTS (TREE_TYPE
> > (view_op0));
> > + // cannot analyse further if nelts is not known
> > + if (!nelts.is_constant ())
> > + return false;
> > +
> > + if (!tree_fits_uhwi_p (idx)
> > + || tree_to_uhwi (idx) > nelts.to_constant ())
> > + return false;
> > + }
> > machine_mode outermode = TYPE_MODE (TREE_TYPE (view_op0));
> > machine_mode extract_mode = TYPE_MODE (TREE_TYPE (ref));
> >
> >
