Re: [PATCH] combine: Don't optimize SIGN_EXTEND of MEM on WORD_REGISTER_OPERATIONS targets [PR113010]

[Greg McGary <gkm@rivosinc.com>]

On 2/1/24 10:24 PM, Jeff Law wrote:
>
> On 2/1/24 18:24, Greg McGary wrote:
>
>> However, for a machine where (WORD_REGISTER_OPERATIONS && 
>> load_extend_op (inner_mode) == SIGN_EXTEND), the high part of a PSoM 
>> is  only known at runtime as 0s or 1s. That's the downstream bug. The 
>> fix for such machines is either (A) forbid static evaluation of the 
>> high part of a PSoM, or (B) forbid transforming (SIGN_EXTEND (MEM 
>> ...) ) into a PSoM. My patch does B. Perhaps you prefer A? The 
>> trouble with A is that in the zero-extend case, it is valid to 
>> statically evaluate as 0. It is only the sign-extend case that isn't 
>> known until runtime. By the time we have a PSoM, its upstream 
>> ancestor as sign- or zero-extend is already lost.
>>
>> Does that give you the understanding you desire, or are there deeper 
>> mysteries to probe?
> It's a good start and I can see what you're trying to do -- and it may 
> in fact be correct -- the quick discussion with Palmer Tuesday and 
> your follow-up have helped a lot).
>
> But just to be sure, what's the incoming rtl at function entry? just 
> "debug_rtx (x)" should be sufficient.

input: (sign_extend:DI (mem/c:SI (symbol_ref:DI ("minus_1") [flags 0x86] 
<var_decl 0x7ffff761cb40 minus_1>) [1 minus_1+0 S4 A32]))

result: (subreg:DI (mem/c:SI (symbol_ref:DI ("minus_1") [flags 0x86] 
<var_decl 0x7ffff761cb40 minus_1>) [1 minus_1+0 S4 A32]) 0)

Later, the high part of the PSoM statically evaluates to 0, the code to 
load and test is elided, and the incorrect alternative is emitted 
unconditionally.

G