Re: [PATCH] Reformat PowerPC movsi_internal
Here is the patch that I committed for movsi_internal: 2019-11-27 Michael Meissner * config/rs6000/rs6000.md (movsi_internal): Reformat. Index: gcc/config/rs6000/rs6000.md === --- gcc/config/rs6000/rs6000.md (revision 278781) +++ gcc/config/rs6000/rs6000.md (working copy) @@ -6889,24 +6889,34 @@ (define_split UNSPEC_MOVSI_GOT))] "") -;; MR LA LWZ LFIWZX LXSIWZX -;; STW STFIWX STXSIWX LI LIS -;; #XXLORXXSPLTIB 0 XXSPLTIB -1 VSPLTISW -;; XXLXOR 0 XXLORC -1P9 const MTVSRWZ MFVSRWZ -;; MF%1 MT%0 NOP +;; MR LA +;; LWZ LFIWZX LXSIWZX +;; STW STFIWX STXSIWX +;; LI LIS # +;; XXLORXXSPLTIB 0 XXSPLTIB -1 VSPLTISW +;; XXLXOR 0 XXLORC -1 P9 const +;; MTVSRWZ MFVSRWZ +;; MF%1 MT%0NOP + (define_insn "*movsi_internal1" [(set (match_operand:SI 0 "nonimmediate_operand" - "=r, r, r, d, v, -m, Z, Z, r, r, -r, wa, wa, wa, v, -wa, v, v, wa, r, -r, *h, *h") + "=r, r, + r, d, v, + m, Z, Z, + r, r, r, + wa, wa, wa, v, + wa, v, v, + wa, r, + r, *h, *h") (match_operand:SI 1 "input_operand" - "r, U, m, Z, Z, -r, d, v, I, L, -n, wa, O, wM, wB, -O, wM, wS, r, wa, -*h, r, 0"))] + "r, U, + m, Z, Z, + r, d, v, + I, L, n, + wa, O, wM, wB, + O, wM, wS, + r, wa, + *h, r, 0"))] "gpc_reg_operand (operands[0], SImode) || gpc_reg_operand (operands[1], SImode)" "@ @@ -6934,23 +6944,32 @@ (define_insn "*movsi_internal1" mt%0 %1 nop" [(set_attr "type" - "*, *, load,fpload, fpload, -store, fpstore, fpstore, *, *, -*, veclogical, vecsimple, vecsimple, vecsimple, -veclogical, veclogical, vecsimple, mffgpr, mftgpr, -*, *, *") + "*, *, + load, fpload, fpload, + store, fpstore,fpstore, + *, *, *, + veclogical, vecsimple, vecsimple, vecsimple, + veclogical, veclogical, vecsimple, + mffgpr, mftgpr, + *, *, *") (set_attr "length" - "*, *, *, *, *, -*, *, *, *, *, -8, *, *, *, *, -*, *, 8, *, *, -*, *, *") + "*, *, + *, *, *, + *, *, *, + *, *, 8, + *, *, *, *, + *, *, 8, + *, *, + *, *, *") (set_attr "isa" - "*, *, *, p8v, p8v, -*, p8v, p8v, *, *, -*, p8v, p9v, p9v, p8v, -p9v,p8v, p9v, p8v, p8v, -*, *, *")]) + "*, *, + *, p8v, p8v, + *, p8v, p8v, + *, *, *, + p8v, p9v, p9v, p8v, + p9v, p8v, p9v, + p8v, p8v, + *, *, *")]) ;; Like movsi, but adjust a SF value to be
Re: [PATCH] Reformat PowerPC movsi_internal
Hi Mike, On Tue, Nov 26, 2019 at 08:32:12PM -0500, Michael Meissner wrote: > As we discussed in the V6 patches #1 and #2, before submitting the patches > adding eI support for movdi and movsi, you prefered that I reformat the > patches > to make them easier in the future to identify the changes. No, that is not the reason. It is to make it easier to handle patches (review, revert, rebase, backport, etc.) A patch should do one thing. If you need to (say) rebase a big patch that merely changes layout, that is not so hard to do (just a bit tedious sometimes). If there are actual changes mixed in... *good luck*! > This patch changes just the movsi_internal insn. All of the constraints are > in > the same order as the current sources. I did add setting "num_insns" to this > patch, but I left in setting "length". I found otherwise the Spec benchmarks > did not generate the same code with the patch. Yes, "length" is still required everywhere, as I said before. This is a big problem. > * config/rs6000/rs6000.md (movsi_internal): Logically align the > columns of constraints and attributes to make it easier to add new > patterns in the middle. Set the num_insns insn attribute. * config/rs6000/rs6000.md (movsi_internal): Reformat. (and the attribute thing, yes, if you have to do that in this same patch. Please don't, in the future.) > --- gcc/config/rs6000/rs6000.md (revision 278746) > +++ gcc/config/rs6000/rs6000.md (working copy) > @@ -6889,24 +6889,36 @@ (define_split >UNSPEC_MOVSI_GOT))] >"") > > -;; MR LA LWZ LFIWZX LXSIWZX > -;; STW STFIWX STXSIWX LI LIS > -;; #XXLORXXSPLTIB 0 XXSPLTIB -1 VSPLTISW > -;; XXLXOR 0 XXLORC -1P9 const MTVSRWZ MFVSRWZ > -;; MF%1 MT%0 NOP > +;; MR LA > +;; LWZ LFIWZX LXSIWZX > +;; STW STFIWX STXSIWX > +;; LI LIS # > +;; XXLORXXSPLTIB 0 XXSPLTIB -1 VSPLTISW > +;; XXLXOR 0 XXLORC -1 P9 const > +;; MTVSRWZ MFVSRWZ > +;; MF%1 MT%0NOP That has broken whitespace (mixing tabs and spaces). All eight leading spaces should be tabs, not just on some lines and not the others. (Similar later, please fix all). > + "*, *, > + load, fpload, fpload, > + store, fpstore,fpstore, > + *, *, *, > + veclogical, vecsimple, vecsimple, vecsimple, > + veclogical, veclogical, vecsimple, > + mffgpr, mftgpr, > + *, *, *") > (set_attr "length" > - "*, *, *, *, *, > - *, *, *, *, *, > - 8, *, *, *, *, > - *, *, 8, *, *, > - *, *, *") > + "*, *, > + *, *, *, > + *, *, *, > + *, *, 8, > + *, *, *, *, > + *, *, 8, > + *, *, *, > + *, *") You have 2 and 3 on the last two lines elsewhere. > + (set_attr "num_insns" > + "*, *, > + *, *, *, > + *, *, *, > + *, *, 2, > + *, *, *, *, > + *, *, 2, > + *, *, > + *, *, *") Different indent on the columns here (that is true elsewhere, too). It would help a lot if you kept all columns the same width, too. Okay for trunk with the changelog and the layout fixed. Thanks! Segher
[PATCH] Reformat PowerPC movsi_internal
As we discussed in the V6 patches #1 and #2, before submitting the patches adding eI support for movdi and movsi, you prefered that I reformat the patches to make them easier in the future to identify the changes. This patch changes just the movsi_internal insn. All of the constraints are in the same order as the current sources. I did add setting "num_insns" to this patch, but I left in setting "length". I found otherwise the Spec benchmarks did not generate the same code with the patch. I have bootstrapped the compiler with this change and the movdi_internal64 patch that will be submitted next, and there were no problems with the bootstrap or tests that regressed. I compared Spec 2017 INT benchmarks and the number of each instruction matches the previous version I tested. Can I check this into the GCC trunk? 2019-11-26 Michael Meissner * config/rs6000/rs6000.md (movsi_internal): Logically align the columns of constraints and attributes to make it easier to add new patterns in the middle. Set the num_insns insn attribute. Index: gcc/config/rs6000/rs6000.md === --- gcc/config/rs6000/rs6000.md (revision 278746) +++ gcc/config/rs6000/rs6000.md (working copy) @@ -6889,24 +6889,36 @@ (define_split UNSPEC_MOVSI_GOT))] "") -;; MR LA LWZ LFIWZX LXSIWZX -;; STW STFIWX STXSIWX LI LIS -;; #XXLORXXSPLTIB 0 XXSPLTIB -1 VSPLTISW -;; XXLXOR 0 XXLORC -1P9 const MTVSRWZ MFVSRWZ -;; MF%1 MT%0 NOP +;; MR LA +;; LWZ LFIWZX LXSIWZX +;; STW STFIWX STXSIWX +;; LI LIS # +;; XXLORXXSPLTIB 0 XXSPLTIB -1 VSPLTISW +;; XXLXOR 0 XXLORC -1 P9 const +;; MTVSRWZ MFVSRWZ +;; MF%1 MT%0NOP + (define_insn "*movsi_internal1" [(set (match_operand:SI 0 "nonimmediate_operand" - "=r, r, r, d, v, -m, Z, Z, r, r, -r, wa, wa, wa, v, -wa, v, v, wa, r, -r, *h, *h") + "=r, r, +r, d, v, +m, Z, Z, +r, r, r, +wa, wa, wa, v, +wa, v, v, +wa, r, + r, *h, *h") + (match_operand:SI 1 "input_operand" - "r, U, m, Z, Z, -r, d, v, I, L, -n, wa, O, wM, wB, -O, wM, wS, r, wa, -*h, r, 0"))] +"r, U, +m, Z, Z, +r, d, v, +I, L, n, +wa, O, wM, wB, +O, wM, wS, +r, wa, +*h, r, 0"))] + "gpc_reg_operand (operands[0], SImode) || gpc_reg_operand (operands[1], SImode)" "@ @@ -6934,23 +6946,41 @@ (define_insn "*movsi_internal1" mt%0 %1 nop" [(set_attr "type" - "*, *, load,fpload, fpload, -store, fpstore, fpstore, *, *, -*, veclogical, vecsimple, vecsimple, vecsimple, -veclogical, veclogical, vecsimple, mffgpr, mftgpr, -*, *, *") + "*, *, + load, fpload, fpload, + store, fpstore,fpstore, + *, *, *, + veclogical, vecsimple, vecsimple, vecsimple, + veclogical, veclogical, vecsimple, + mffgpr, mftgpr, + *, *, *") (set_attr "length" - "*, *, *, *, *, -*, *, *, *, *, -8, *, *, *, *, -*, *, 8, *, *, -*, *, *") + "*, *, + *, *, *, + *, *,
