Re: [PATCH], V7, #2 of 7, Use PLI to load up 32-bit SImode constants
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
Re: [PATCH], V7, #2 of 7, Use PLI to load up 32-bit SImode constants
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) >= 0x1 > > > > - && (INTVAL (operands[1]) & 0x) != 0" > > > > + && (INTVAL (operands[1]) & 0x) != 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. -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
Re: [PATCH], V7, #2 of 7, Use PLI to load up 32-bit SImode constants
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) >= 0x1 > > > - && (INTVAL (operands[1]) & 0x) != 0" > > > + && (INTVAL (operands[1]) & 0x) != 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
Re: [PATCH], V7, #2 of 7, Use PLI to load up 32-bit SImode constants
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) >= 0x1 > > - && (INTVAL (operands[1]) & 0x) != 0" > > + && (INTVAL (operands[1]) & 0x) != 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. -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
Re: [PATCH], V7, #2 of 7, Use PLI to load up 32-bit SImode constants
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) >= 0x1 > - && (INTVAL (operands[1]) & 0x) != 0" > + && (INTVAL (operands[1]) & 0x) != 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
[PATCH], V7, #2 of 7, Use PLI to load up 32-bit SImode constants
This patch generates the PLI (PADDI) instruction to load up 32-bit SImode constants on the future system. It adds an alternative to movsi, and prevents the movsi load immediate from being split. I have built compilers with this patch which bootstrapped fine, and there were no regressions in the test suite. Can I check this into the trunk? 2019-11-14 Michael Meissner * config/rs6000/rs6000.md (movsi_internal1): Add support to load up 32-bit SImode integer constants with PADDI. (movsi integer constant splitter): Do not split constant if PADDI can load it up directly. 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 -;; #XXLORXXSPLTIB 0 XXSPLTIB -1 VSPLTISW -;; XXLXOR 0 XXLORC -1P9 const MTVSRWZ MFVSRWZ -;; MF%1 MT%0 NOP +;; PLI #XXLORXXSPLTIB 0 XXSPLTIB -1 +;; VSPLTISW XXLXOR 0 XXLORC -1P9 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
