Redundant sign-extension instructions on RISC-V

[Richard Biener]

On Wed, Aug 30, 2017 at 11:53 AM, Michael Clark <michaeljclark@mac.com> wrote:
>
>> On 30 Aug 2017, at 9:43 PM, Michael Clark <michaeljclark@mac.com> wrote:
>>
>>>> diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
>>>> index ce632ae..25dd70f 100644
>>>> --- a/gcc/simplify-rtx.c
>>>> +++ b/gcc/simplify-rtx.c
>>>> @@ -1503,6 +1503,10 @@ simplify_unary_operation_1 (enum rtx_code code, machine_mode mode, rtx op)
>>>>     /* (sign_extend:M (lshiftrt:N <X> (const_int I))) is better as
>>>>        (zero_extend:M (lshiftrt:N <X> (const_int I))) if I is not 0.  */
>>>>     if (GET_CODE (op) == LSHIFTRT
>>>> +#if defined(POINTERS_EXTEND_UNSIGNED)
>>>> +      /* we skip this optimisation if pointers naturally extend signed */
>>>> +         && POINTERS_EXTEND_UNSIGNED
>>>> +#endif
>>>>        && CONST_INT_P (XEXP (op, 1))
>>>>        && XEXP (op, 1) != const0_rtx)
>>>>      return simplify_gen_unary (ZERO_EXTEND, mode, op, GET_MODE (op));
>>>
>>> Is it just me or does this miss a || mode != Pmode || GET_MODE (op) != ptr_mode
>>> check?  Note the comment says exactly the opposite as the transform...
>>>
>>> I’m not even sure why this simplification is correct in the first place?!
>>
>> I hope you are not confusing my use of POINTERS_EXTEND_UNSIGNED as a proxy for the property that defines whether sub width operations sign-extend to the full width of the register vs zero extend. Are you taking about our added comment?

I'm talking about using POINTERS_EXTEND_UNSIGNED for sth that looks
unrelated (and that has no easy way to be queried as you noted).

Maybe TARGET_MODE_REP_EXTENDED is suitable here?  So
ask
TARGET_MODE_REP_EXTENDED (GET_MODE (op), mode)) == SIGN_EXTEND
and if that's true skip the optimization?

Richard.

> Read it as:
>
> riscv.h #define REGISTERS_EXTEND_UNSIGNED false
>
> +#if defined(REGISTERS_EXTEND_UNSIGNED)
> +      /* we skip this optimisation if registers naturally extend signed */
> +         && REGISTERS_EXTEND_UNSIGNED
> +#endif
>
> The comment is the inverse because RISC-V registers naturally extend signed and REGISTERS_EXTEND_UNSIGNED is false. GCC defines unsigned false for RISC-V promotions.