On 09/06/2017 09:53 AM, Jeff Law wrote:
>> I think the easiest solution to this is for combine to notice when IOR has
>> operands with non-zero-bits that do not overlap, convert the operation to
>> ADD.
>> That allows the final two insns to fold to "addw" and the compiler need do no
>> further analysis.
> I thought we had combine support for that. I don't think it's ever been
> particularly good though. With your alpha background you're probably
> more familiar with it than anyone -- IIRC it fell out of removal of low
> order bit masking to deal with alignment issues on the Alpha.
Yes, but that would have been within AND to drop unnecessary/redundant masks.
We probably just need a few lines over in IOR to handle this case with the same
machinery.
Heh... Just having a browse shows that we currently perform exactly the
opposite transformation:
/* If we are adding two things that have no bits in common, convert
the addition into an IOR. This will often be further simplified,
for example in cases like ((a & 1) + (a & 2)), which can
become a & 3. */
So managing these conflicting goals might be tricky...
> I wrote some match.pd patterns to do it in gimple as part of a larger
> problem. The work as a whole on that larger problem ultimately didn't
> pan out (generated even worse code than what we have on the trunk). But
> it might be possible to resurrect those patterns and see if they are
> useful independently. My recollection was I was looking at low order
> bits only, but the concepts were general enough.
That would be interesting, yes.
>>> $ cat rshift.c
>>> unsigned rs24(unsigned rs1) { return rs1 >> 24; }
>>> $ cat rshift.s
>>> rs24:
>>> sllw a0,a0,24
>>> sext.w a0,a0 # redundant
>>> ret
>>
>> That seems like a missing check somewhere (combine? simplify-rtx? both?) for
>> SUBREG_PROMOTED_SIGNED_P. Since Alpha didn't have a 32-bit shift you're in
>> new
>> territory for this one.
> Yea. I'd also expect zero/nonzero bits tracking in combine to catch
> this. Shouldn't the sign bit be known to be zero after the shift which
> makes the extension redundant regardless of the SUBREG_PROMOTED flag?
You're right, this should be irrelevant. Any anyway combine should be
constrained by modes, so it should require the SIGN_EXTEND to be present just
to make the modes match up. Perhaps this test case is being suppressed because
of something else, e.g. failure to combine insns when a hard-reg is involved?
r~