Hi Segher!
on 2022/6/28 00:02, Segher Boessenkool wrote:
> Hi!
>
> On Mon, Jun 27, 2022 at 05:35:03PM +0800, Kewen.Lin wrote:
>> -(define_insn "*rotl<mode>3_insert_4"
>> - [(set (match_operand:GPR 0 "gpc_reg_operand" "=r")
>> - (ior:GPR (and:GPR (match_operand:GPR 3 "gpc_reg_operand" "0")
>> - (match_operand:GPR 4 "const_int_operand" "n"))
>> - (lshiftrt:GPR (match_operand:GPR 1 "gpc_reg_operand" "r")
>> +; It's unable to extend this define_insn for DImode, because the
>> +; corresponding hardware instruction for DImode is rldimi, which only has
>> +; four operands and the end of mask is always (63 - SH) rather than 63
>> +; that is required for this pattern.
>> +(define_insn "*rotlsi3_insert_4"
>> + [(set (match_operand:SI 0 "gpc_reg_operand" "=r")
>> + (ior:SI (and:SI (match_operand:SI 3 "gpc_reg_operand" "0")
>> + (match_operand:SI 4 "const_int_operand" "n"))
>> + (lshiftrt:SI (match_operand:SI 1 "gpc_reg_operand" "r")
>> (match_operand:SI 2 "const_int_operand" "n"))))]
>
> (Broken indentation here, on all of the last three lines differently
> wrong:
>
> [(set (match_operand:SI 0 "gpc_reg_operand" "=r")
> (ior:SI (and:SI (match_operand:SI 3 "gpc_reg_operand" "0")
> (match_operand:SI 4 "const_int_operand" "n"))
> (lshiftrt:SI (match_operand:SI 1 "gpc_reg_operand" "r")
> (match_operand:SI 2 "const_int_operand" "n"))))]
>
> is better.)
>
Thanks for catching, I forgot to re-indent after substituting. :(
>> - "<MODE>mode == SImode &&
>> - GET_MODE_PRECISION (<MODE>mode)
>> - == INTVAL (operands[2]) + exact_log2 (-UINTVAL (operands[4]))"
>> -{
>> - operands[2] = GEN_INT (GET_MODE_PRECISION (<MODE>mode)
>> - - INTVAL (operands[2]));
>> - if (<MODE>mode == SImode)
>> - return "rlwimi %0,%1,%h2,32-%h2,31";
>> - else
>> - return "rldimi %0,%1,%H2,64-%H2";
>> -}
>> + "INTVAL (operands[2]) + exact_log2 (-UINTVAL (operands[4])) == 32"
>> + "rlwimi %0,%1,32-%h2,%h2,31"
>> [(set_attr "type" "insert")])
>
> I was going to say this is not an improvement at all, because now you
> need that big comment. There are many tricky details here, more
> important ones than the comment talks about. It is better to show such
> things in the code instead.
>
> But it already does show that :-) The patch is okay with that four-line
> comment deleted (and the indentation fixed). Thanks!
>
Thanks! It's adjusted as your comments and committed in r13-1313.
BR,
Kewen
