Hi Steve,
>> After special cases you could do something like t = mask2 + (HWI_1U <<
>> shift);
>> return t == (t & -t) to check for a valid bfi.
>
> I am not sure I follow this logic and my attempts to use this did not
> work so I kept my original code.
It's similar to the initial code in aarch64_bitmask_imm, but rather than adding
the lowest bit to the value to verify it is a mask (val + (val & -val)), we use
the
shift instead. If the shift is exactly right, it reaches the first set bit of
the mask.
Adding the low bit to a valid mask always results in zero or a single set bit.
The standard idiom to check that is t == (t & -t).
>> + "bfi\t%<GPI:w>0, %<GPI:w>1, 0, %P2"
>>
>> This could emit a width that may be 32 too large in SImode if bit 31 is set
>> (there is another use of %P in aarch64.md which may have the same
>> issue).
>
> I am not sure why having bit 31 set would be a problem. Sign
> extension?
Yes, if bit 31 is set, %P will emit 33 for a 32-bit constant which is obviously
wrong.
Your patch avoids this for bfi by explicitly computing the correct value.
This looks good to me (and creates useful bfi's as expected), but I can't
approve.
Wilco
