Re: Redundant sign-extension instructions on RISC-V

[Jeff Law <law at redhat dot com>]

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