Message ID | 91690387-7e70-3e5c-9a20-00863dc393e3@linux.ibm.com |
---|---|
State | New |
Headers | show |
Series | rs6000: Simplify *rotl<mode>3_insert_4 by removing DImode | expand |
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.) > - "<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! Segher
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
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md index c55ee7e171a..aef88e94576 100644 --- a/gcc/config/rs6000/rs6000.md +++ b/gcc/config/rs6000/rs6000.md @@ -4206,23 +4206,18 @@ (define_split operands[4] = GEN_INT ((HOST_WIDE_INT_1U << INTVAL (operands[2])) - 1); }) -(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"))))] - "<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")]) (define_insn "*rotlsi3_insert_5"