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). > 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, Andre