Re: [PATCH] Strip of a vector load which is only used partially.

[Richard Biener <richard.guenther@gmail.com>]

On Tue, May 10, 2022 at 12:58 AM Jeff Law via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
>
> On 5/5/2022 2:26 AM, Richard Biener via Gcc-patches wrote:
> > On Thu, May 5, 2022 at 7:04 AM liuhongt <hongtao.liu@intel.com> wrote:
> >> Optimize
> >>
> >>    _1 = *srcp_3(D);
> >>    _4 = VEC_PERM_EXPR <_1, _1, { 4, 5, 6, 7, 4, 5, 6, 7 }>;
> >>    _5 = BIT_FIELD_REF <_4, 128, 0>;
> >>
> >> to
> >>
> >>    _1 = *srcp_3(D);
> >>    _5 = BIT_FIELD_REF <_1, 128, 128>;
> >>
> >> the upper will finally be optimized to
> >>
> >> _5 = BIT_FIELD_REF <*srcp_3(D), 128, 128>;
> >>
> >> Bootstrapped and regtested on x86_64-pc-linux-gnu{m32,}.
> >> Ok for trunk?
> > Hmm, tree-ssa-forwprop.cc:simplify_bitfield_ref should already
> > handle this in the
> >
> >    if (code == VEC_PERM_EXPR
> >        && constant_multiple_p (bit_field_offset (op), size, &idx))
> >      {
> >
> > part of the code - maybe that needs to be enhanced to cover
> > a contiguous stride in the VEC_PERM_EXPR.  I see
> > we have
> >
> >    size = TREE_INT_CST_LOW (TYPE_SIZE (elem_type));
> >    if (maybe_ne (bit_field_size (op), size))
> >      return false;
> >
> > where it will currently bail, so adjust that to check for a
> > constant multiple.  I also think we should only handle the
> > case where the new bit_field_offset alignment is not
> > worse than the original one.
> >
> > That said, I'd prefer if you integrate this transform with
> > simplify_bitfield_ref.
> I've got a hack here that tries to do something similar, but it's trying
> to catch the case where we CONSTRUCTOR feeds the BIT_FIELD_REF.  It
> walks the CONSTRUCTOR elements to see if an element has the right
> offset/size to satisify the BIT_FIELD_REF. For x264 we're often able to
> eliminate the VEC_PERMUTE entirely and just forward operands into the
> BIT_FIELD_REF.
>
> I was leaning towards moving those bits into match.pd before submitting,
> but if you'd prefer them in tree-ssa-forwprop, that's even easier.

I think when deciding where to put things it's important to look where related
transforms reside.  We already do have a (simplify (BIT_FIELD_REF
CONSTRUCTOR@ ...))
pattern which should also handle your case already.  So instead of
adding something
new it would be nice to figure why it doesn't handle the case you are
interested in and
eventually just adjust the existing pattern.

In the case of the above patch there isn't a match.pd pattern for this yet but
forwprop already has code to match bit-field-refs with vec-perms, so that's the
reason I preferred extending that.  But of course the whole thing could live in
match.pd as well.

Richard.

> Jeff
>
>