Le 02/03/2023 à 18:18, Andrew Stubbs a écrit :
> On 01/03/2023 16:56, Paul-Antoine Arras wrote:
>> This patch introduces instruction patterns for conditional min and max
>> operations (cond_{f|s|u}{max|min}) in the GCN machine description. It 
>> also allows the exec register to be saved in SGPRs to avoid spilling 
>> to memory.
>> Tested on GCN3 Fiji gfx803.
>>
>> OK for trunk?
> 
> Not quite yet, but it's only a few cosmetic issues, I think.
> 
>> +(define_insn_and_split "<expander><mode>3"
>> +  [(set (match_operand:V_DI 0 "register_operand"      "=  v")
>> +    (minmaxop:V_DI
>> +      (match_operand:V_DI 1 "gcn_alu_operand"     "%  v")
>> +          (match_operand:V_DI 2 "gcn_alu_operand"     "   v")))
>> +    (clobber (reg:DI VCC_REG))]
> 
> No need to make it commutative when the two operands have the same 
> constraints. There's a few more instances of this later.
> 
>> +    if (<code> == smin || <code> == smax)
>> +      emit_insn (gen_vec_cmp<mode>di (vcc, minp ? gen_rtx_LT 
>> (VOIDmode, 0, 0) :
>> +                                    gen_rtx_GT (VOIDmode, 0, 0), 
>> operands[1], operands[2]));
>> +    else
>> +      emit_insn (gen_vec_cmp<mode>di (vcc, minp ? gen_rtx_LTU 
>> (VOIDmode, 0, 0) :
>> +                                    gen_rtx_GTU (VOIDmode, 0, 0), 
>> operands[1], operands[2]));
>> +
> 
> Long lines need to be wrapped, here and elsewhere.

The amended patch attached should fix those issues. Let me know if it 
looks good to you.
-- 
PA