Re: [PATCH] Teach vectorizer to deal with bitfield accesses (was: [RFC] Teach vectorizer to deal with bitfield reads)

[Richard Biener <rguenther@suse.de>]

On Mon, 26 Sep 2022, Andre Vieira (lists) wrote:

> 
> On 08/09/2022 12:51, Richard Biener wrote:
> >
> > I'm curious, why the push to redundant_ssa_names?  That could use
> > a comment ...
> So I purposefully left a #if 0 #else #endif in there so you can see the two
> options. But the reason I used redundant_ssa_names is because ifcvt seems to
> use that as a container for all pairs of (old, new) ssa names to replace
> later. So I just piggy backed on that. I don't know if there's a specific
> reason they do the replacement at the end? Maybe some ordering issue? Either
> way both adding it to redundant_ssa_names or doing the replacement inline work
> for the bitfield lowering (or work in my testing at least).

Possibly because we (in the past?) inserted/copied stuff based on
predicates generated at analysis time after we decide to elide something
so we need to watch for later appearing uses.  But who knows ... my mind
fails me here.

If it works to replace uses immediately please do so.  But now
I wonder why we need this - the value shouldn't change so you
should get away with re-using the existing SSA name for the final value?

> > Note I fear we will have endianess issues when translating
> > bit-field accesses to BIT_FIELD_REF/INSERT and then to shifts.  Rules
> > for memory and register operations do not match up (IIRC, I repeatedly
> > run into issues here myself).  The testcases all look like they
> > won't catch this - I think an example would be sth like
> > struct X { unsigned a : 23; unsigned b : 9; }, can you see to do
> > testing on a big-endian target?
> I've done some testing and you were right, it did fall apart on big-endian. I
> fixed it by changing the way we compute the 'shift' value and added two extra
> testcases for read and write each.
> >
> > Sorry for the delay in reviewing.
> No worries, apologies myself for the delay in reworking this, had a nice
> little week holiday in between :)
> 
> I'll write the ChangeLogs once the patch has stabilized.

Thanks,
Richard.