Re: [PATCH] RISC-V: Add min/max patterns for ifcvt.

[Jeff Law <jeffreyalaw@gmail.com>]

On 6/3/24 11:03 AM, Palmer Dabbelt wrote:

>>
>> +;; Provide a minmax pattern for ifcvt to match.
>> +(define_insn "*<bitmanip_minmax_cmp_insn>_cmp_<mode>3"
>> +  [(set (match_operand:X 0 "register_operand" "=r")
>> +    (if_then_else:X
>> +        (bitmanip_minmax_cmp_op
>> +        (match_operand:X 1 "register_operand" "r")
>> +        (match_operand:X 2 "register_operand" "r"))
>> +        (match_dup 1)
>> +        (match_dup 2)))]
>> +  "TARGET_ZBB"
>> +  "<bitmanip_minmax_cmp_insn>\t%0,%1,%z2"
>> +  [(set_attr "type" "<bitmanip_minmax_cmp_insn>")])
> 
> This is a bit different than how we're handling the other min/max type 
> attributes
> 
>     (define_insn "*<bitmanip_optab><mode>3"
>       [(set (match_operand:X 0 "register_operand" "=r")
>             (bitmanip_minmax:X (match_operand:X 1 "register_operand" "r")
>                    (match_operand:X 2 "reg_or_0_operand" "rJ")))]
>       "TARGET_ZBB"
>       "<bitmanip_insn>\t%0,%1,%z2"
>       [(set_attr "type" "<bitmanip_insn>")])
> 
> but it looks like it ends up with the same types after all the iterators 
> (there's some "max vs smax" and "smax vs maxs" juggling, but IIUC it 
> ends up in the same place).  I think it'd be clunkier to try and use all 
> the same iterators, though, so
> 
> Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
> 
> [I was wondering if we need the reversed, Jeff on the call says we 
> don't.  I couldn't figure out how to write a test for it.]
Right.  I had managed to convince myself that we didn't need the 
reversed case.  I'm less sure now than I was earlier, but I'm also 
confident that if the need arises we can trivially handle it.  At some 
point there's canonicalization of the condition and that's almost 
certainly what's making it hard to synthesize a testcase for the 
reversed pattern.

The other thing I pondered was whether or not we should support SImode 
min/max on rv64.  It was critical for simplifying that abs2 routine in 
x264, but I couldn't convince myself it was needed here.  So I just set 
it aside and didn't mention it.

jeff