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

[Greg McGary <gkm@rivosinc.com>]

On 2/4/24 9:58 PM, Jeff Law wrote:
>
> On 2/2/24 15:48, Greg McGary wrote:
>>
>> 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.
> So I think we need to know where that high part gets statically turned 
> into a zero.
>
> I'm not happy with the sign_extend->subreg transformation as we 
> generally want to avoid (subreg (mem)) for various reasons.  So we'll 
> probably want to do something like your patch as well.   But let's 
> chase down the static evaluation of the high part to zero first -- 
> that's clearly wrong given the defined semantics of (subreg (mem)) in 
> the presence of LOAD_EXTEND_OP.


This prevents the buggy evaluation. What think ye?

diff --git a/gcc/combine.cc b/gcc/combine.cc
index 812553c091e..736206242e1 100644
--- a/gcc/combine.cc
+++ b/gcc/combine.cc
@@ -12550,9 +12550,15 @@ simplify_comparison (enum rtx_code code, rtx 
*pop0, rtx *pop1)
             }

           /* If the inner mode is narrower and we are extracting the 
low part,
-            we can treat the SUBREG as if it were a ZERO_EXTEND. */
+            we can treat the SUBREG as if it were a ZERO_EXTEND ...  */
           if (paradoxical_subreg_p (op0))
-           ;
+           {
+             /* ... except we can't treat as ZERO_EXTEND when a machine
+                automatically sign-extends loads. */
+             if (MEM_P (SUBREG_REG (op0)) && WORD_REGISTER_OPERATIONS
+                 && load_extend_op (inner_mode) == SIGN_EXTEND)
+               break;
+           }
           else if (subreg_lowpart_p (op0)
                    && GET_MODE_CLASS (mode) == MODE_INT
                    && is_int_mode (GET_MODE (SUBREG_REG (op0)), 
&inner_mode)