On 09/06/2017 11:17 AM, Richard Henderson wrote:
> 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.
Right. My point was most of the infrastructure ought to be in place.
>
> 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...
And I wonder how often that happens in practice.
>
>> 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.
So I just dug up the BZ. 59393. Sadly it was the other way around --
turning a PLUS into BIT_IOR like combine. So not helpful here.
The PLUS->IOR apparently was still profitable from a codesize
standpoint. But in general, I'm not sure how to select between the two
forms. I guess IOR is slightly better because we know precisely what
bits are potentially changed based on the constant.
>
>>>> $ 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?
It could well be hard register involvement.
jeff
