| Message ID | 20191114224243.GB7528@ibm-toto.the-meissners.org |
|---|---|
| State | New |
| Headers | show |
| Series | , V7, #2 of 7, Use PLI to load up 32-bit SImode constants | expand |
On Thu, Nov 14, 2019 at 05:42:43PM -0500, Michael Meissner wrote: > -;; Split a load of a large constant into the appropriate two-insn > -;; sequence. > +;; Split a load of a large constant into the appropriate two-insn sequence. On > +;; systems that support PADDI (PLI), we can use PLI to load any 32-bit constant > +;; in one instruction. > > (define_split > [(set (match_operand:SI 0 "gpc_reg_operand") > (match_operand:SI 1 "const_int_operand"))] > "(unsigned HOST_WIDE_INT) (INTVAL (operands[1]) + 0x8000) >= 0x10000 > - && (INTVAL (operands[1]) & 0xffff) != 0" > + && (INTVAL (operands[1]) & 0xffff) != 0 && !TARGET_PREFIXED_ADDR" > [(set (match_dup 0) > (match_dup 2)) > (set (match_dup 0) Please use num_insns_constant, instead (and fix num_insns_constant_gpr so it knows about SIGNED_34BIT). Segher
On Fri, Nov 22, 2019 at 06:20:52PM -0600, Segher Boessenkool wrote: > On Thu, Nov 14, 2019 at 05:42:43PM -0500, Michael Meissner wrote: > > -;; Split a load of a large constant into the appropriate two-insn > > -;; sequence. > > +;; Split a load of a large constant into the appropriate two-insn sequence. On > > +;; systems that support PADDI (PLI), we can use PLI to load any 32-bit constant > > +;; in one instruction. > > > > (define_split > > [(set (match_operand:SI 0 "gpc_reg_operand") > > (match_operand:SI 1 "const_int_operand"))] > > "(unsigned HOST_WIDE_INT) (INTVAL (operands[1]) + 0x8000) >= 0x10000 > > - && (INTVAL (operands[1]) & 0xffff) != 0" > > + && (INTVAL (operands[1]) & 0xffff) != 0 && !TARGET_PREFIXED_ADDR" > > [(set (match_dup 0) > > (match_dup 2)) > > (set (match_dup 0) > > Please use num_insns_constant, instead (and fix num_insns_constant_gpr > so it knows about SIGNED_34BIT). The previous patch V6 #1 already had the modification for num_insns_constant_gpr.
On Mon, Nov 25, 2019 at 05:17:08PM -0500, Michael Meissner wrote: > On Fri, Nov 22, 2019 at 06:20:52PM -0600, Segher Boessenkool wrote: > > > (define_split > > > [(set (match_operand:SI 0 "gpc_reg_operand") > > > (match_operand:SI 1 "const_int_operand"))] > > > "(unsigned HOST_WIDE_INT) (INTVAL (operands[1]) + 0x8000) >= 0x10000 > > > - && (INTVAL (operands[1]) & 0xffff) != 0" > > > + && (INTVAL (operands[1]) & 0xffff) != 0 && !TARGET_PREFIXED_ADDR" > > > [(set (match_dup 0) > > > (match_dup 2)) > > > (set (match_dup 0) > > > > Please use num_insns_constant, instead (and fix num_insns_constant_gpr > > so it knows about SIGNED_34BIT). > > The previous patch V6 #1 already had the modification for > num_insns_constant_gpr. It's not in trunk yet? Segher
On Mon, Nov 25, 2019 at 06:49:49PM -0600, Segher Boessenkool wrote: > On Mon, Nov 25, 2019 at 05:17:08PM -0500, Michael Meissner wrote: > > On Fri, Nov 22, 2019 at 06:20:52PM -0600, Segher Boessenkool wrote: > > > > (define_split > > > > [(set (match_operand:SI 0 "gpc_reg_operand") > > > > (match_operand:SI 1 "const_int_operand"))] > > > > "(unsigned HOST_WIDE_INT) (INTVAL (operands[1]) + 0x8000) >= 0x10000 > > > > - && (INTVAL (operands[1]) & 0xffff) != 0" > > > > + && (INTVAL (operands[1]) & 0xffff) != 0 && !TARGET_PREFIXED_ADDR" > > > > [(set (match_dup 0) > > > > (match_dup 2)) > > > > (set (match_dup 0) > > > > > > Please use num_insns_constant, instead (and fix num_insns_constant_gpr > > > so it knows about SIGNED_34BIT). > > > > The previous patch V6 #1 already had the modification for > > num_insns_constant_gpr. > > It's not in trunk yet? Sorry, the USA Thanksgiving holiday got in the way. No, the change for num_insns_constant_gpr could not go in until the support for PLI went in (patch V6 #1). So for patches V6 #1-3, I believe you approved #1 (movdi) and #3 (addi3/addsi3) after the changes but not #2 (movsi). I haven't yet applied the specific bits of #1 or #3 after doing the reformating patch. Are the bits for #2 ok with the above change to use num_insns_constant_gpr. Or did you want to see the 3 patches once again after the reformating.
On Tue, Dec 03, 2019 at 12:57:24PM -0500, Michael Meissner wrote: > No, the change for num_insns_constant_gpr could not go in until the support for > PLI went in (patch V6 #1). Well, I lost track. So your version 7 to 9 patches do *not* replace the v6 patches? Or does "V" mean something else? Please post patches you propose currently (after addressing comments, saying what is changed wrt the previous submission, etc.) And if you restructure the patch sets, e.g. add new things to patches, I will have to start reviewing from scratch. While if you just do some modifications (and you say what you modified, and that matches reality), and those modifications are in line with previous reviews, then reviewing makes easy progress. It also normally would not take a single day even to make such changes. > So for patches V6 #1-3, I believe you approved #1 (movdi) and #3 (addi3/addsi3) > after the changes but not #2 (movsi). I haven't yet applied the specific bits > of #1 or #3 after doing the reformating patch. Are the bits for #2 ok with the > above change to use num_insns_constant_gpr. Or did you want to see the 3 > patches once again after the reformating. https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01605.html This is six weeks ago. And I do not know if what you call "v6" is this. Segher
Index: gcc/config/rs6000/rs6000.md =================================================================== --- gcc/config/rs6000/rs6000.md (revision 278175) +++ gcc/config/rs6000/rs6000.md (working copy) @@ -6891,22 +6891,22 @@ (define_split ;; MR LA LWZ LFIWZX LXSIWZX ;; STW STFIWX STXSIWX LI LIS -;; # XXLOR XXSPLTIB 0 XXSPLTIB -1 VSPLTISW -;; XXLXOR 0 XXLORC -1 P9 const MTVSRWZ MFVSRWZ -;; MF%1 MT%0 NOP +;; PLI # XXLOR XXSPLTIB 0 XXSPLTIB -1 +;; VSPLTISW XXLXOR 0 XXLORC -1 P9 const MTVSRWZ +;; MFVSRWZ MF%1 MT%0 NOP (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, 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"))] + eI, 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)" "@ @@ -6920,6 +6920,7 @@ (define_insn "*movsi_internal1" stxsiwx %x1,%y0 li %0,%1 lis %0,%v1 + li %0,%1 # xxlor %x0,%x1,%x1 xxspltib %x0,0 @@ -6936,21 +6937,21 @@ (define_insn "*movsi_internal1" [(set_attr "type" "*, *, load, fpload, fpload, store, fpstore, fpstore, *, *, - *, veclogical, vecsimple, vecsimple, vecsimple, - veclogical, veclogical, vecsimple, mffgpr, mftgpr, - *, *, *") + *, *, 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, - *, *, *")]) + fut, *, p8v, p9v, p9v, + p8v, p9v, p8v, p9v, p8v, + p8v, *, *, *")]) ;; Like movsi, but adjust a SF value to be used in a SI context, i.e. ;; (set (reg:SI ...) (subreg:SI (reg:SF ...) 0)) @@ -7095,14 +7096,15 @@ (define_insn "*movsi_from_df" "xscvdpsp %x0,%x1" [(set_attr "type" "fp")]) -;; Split a load of a large constant into the appropriate two-insn -;; sequence. +;; Split a load of a large constant into the appropriate two-insn sequence. On +;; systems that support PADDI (PLI), we can use PLI to load any 32-bit constant +;; in one instruction. (define_split [(set (match_operand:SI 0 "gpc_reg_operand") (match_operand:SI 1 "const_int_operand"))] "(unsigned HOST_WIDE_INT) (INTVAL (operands[1]) + 0x8000) >= 0x10000 - && (INTVAL (operands[1]) & 0xffff) != 0" + && (INTVAL (operands[1]) & 0xffff) != 0 && !TARGET_PREFIXED_ADDR" [(set (match_dup 0) (match_dup 2)) (set (match_dup 0)