Re: Redundant sign-extension instructions on RISC-V

[Richard Henderson <rth at twiddle dot net>]

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~