On Thu, Nov 7, 2024 at 8:25 AM Tejas Belagod <tejas.bela...@arm.com> wrote:
>
> On 11/6/24 6:02 PM, Richard Biener wrote:
> > On Wed, Nov 6, 2024 at 12:49 PM Tejas Belagod <tejas.bela...@arm.com> wrote:
> >>
> >> Ensure sizeless types don't end up trying to be canonicalised to
> >> BIT_FIELD_REFs.
> >
> > You mean variable-sized? But don't we know, when there's a constant
> > array index,
> > that the size is at least so this indexing is OK? So what's wrong with a
> > fixed position, fixed size BIT_FIELD_REF extraction of a VLA object?
> >
> > Richard.
> >
>
> Ah! The code and comment/description don't match, sorry. This change
> started out as gating out all canonicalizations of VLA vectors when I
> had limited understanding of how this worked, but eventually was
> simplified to gate in only those offsets that were known_le, but missed
> out fixing the comment/description. So, for eg.
>
> int foo (svint32_t v) { return v[3]; }
>
> canonicalises to a BIT_FIELD_REF <v, 32, 96>
>
> but something like:
>
> int foo (svint32_t v) { return v[4]; }
So this is possibly out-of-bounds?
> reduces to a VEC_EXTRACT <>
But if out-of-bounds a VEC_EXTRACT isn't any better than a BIT_FIELD_REF, no?
> I'll fix the comment/description.
>
> Thanks,
> Tejas.
>
> >> gcc/ChangeLog:
> >>
> >> * gimple-fold.cc (maybe_canonicalize_mem_ref_addr): Disallow
> >> sizeless
> >> types in BIT_FIELD_REFs.
> >> ---
> >> gcc/gimple-fold.cc | 3 ++-
> >> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
> >> index c19dac0dbfd..dd45d9f7348 100644
> >> --- a/gcc/gimple-fold.cc
> >> +++ b/gcc/gimple-fold.cc
> >> @@ -6281,6 +6281,7 @@ maybe_canonicalize_mem_ref_addr (tree *t, bool
> >> is_debug = false)
> >> && VECTOR_TYPE_P (TREE_TYPE (TREE_OPERAND (TREE_OPERAND (*t, 0),
> >> 0))))
> >> {
> >> tree vtype = TREE_TYPE (TREE_OPERAND (TREE_OPERAND (*t, 0), 0));
> >> + /* BIT_FIELD_REF can only happen on constant-size vectors. */
> >> if (VECTOR_TYPE_P (vtype))
> >> {
> >> tree low = array_ref_low_bound (*t);
> >> @@ -6294,7 +6295,7 @@ maybe_canonicalize_mem_ref_addr (tree *t, bool
> >> is_debug = false)
> >> (TYPE_SIZE (TREE_TYPE (*t))));
> >> widest_int ext
> >> = wi::add (idx, wi::to_widest (TYPE_SIZE (TREE_TYPE
> >> (*t))));
> >> - if (wi::les_p (ext, wi::to_widest (TYPE_SIZE (vtype))))
> >> + if (known_le (ext, wi::to_poly_widest (TYPE_SIZE
> >> (vtype))))
> >> {
> >> *t = build3_loc (EXPR_LOCATION (*t), BIT_FIELD_REF,
> >> TREE_TYPE (*t),
> >> --
> >> 2.25.1
> >>
>
