On Fri, Nov 22, 2019 at 06:06:19PM -0600, Segher Boessenkool wrote:
> On Thu, Nov 14, 2019 at 05:40:10PM -0500, Michael Meissner wrote:
> > --- gcc/config/rs6000/rs6000.c (revision 278173)
> > +++ gcc/config/rs6000/rs6000.c (working copy)
> > @@ -5552,7 +5552,7 @@ static int
> > num_insns_constant_gpr (HOST_WIDE_INT value)
> > {
> > /* signed constant loadable with addi */
> > - if (((unsigned HOST_WIDE_INT) value + 0x8000) < 0x10000)
> > + if (SIGNED_16BIT_OFFSET_P (value))
> > return 1;
>
> Hrm, so the SIGNED_nBIT_OFFSET_P should not be called "offset", if we use
> them for other numbers as well.
>
> > -;; GPR store GPR load GPR move GPR li GPR lis
> > GPR #
> > -;; FPR store FPR load FPR move AVX store AVX store
> > AVX load
> > -;; AVX load VSX move P9 0 P9 -1 AVX 0/-1
> > VSX 0
> > -;; VSX -1 P9 const AVX const From SPR To SPR
> > SPR<->SPR
> > -;; VSX->GPR GPR->VSX
> > +;; GPR store GPR load GPR move GPR li GPR lis
> > GPR pli
> > +;; GPR # FPR store FPR load FPR move AVX store
> > AVX store
> > +;; AVX load AVX load VSX move P9 0 P9 -1
> > AVX 0/-1
> > +;; VSX 0 VSX -1 P9 const AVX const From SPR To
> > SPR
> > +;; SPR<->SPR VSX->GPR GPR->VSX
>
> I cannot make heads or tails of it this way. Please just add the "pli",
> don't rearrange everything else.
You have to put PLI after LI and LIS and before the splitter insn.
> There do not have to be exactly six per line. The only reason to have
> some order here is to make it easier to read, not to make it *harder*!
But in doing the transformation, I will modify all of the constraint and
attribute lines. Otherwise it makes it impossible to add new things, and it
makes it impossible to find the appropriate insn.
> So for this first line let's have three GPR moves, and then have four
> load immediates. Then in the future if we need to edit it again, make
> the edited part make some sense, etc.
So right now we have:
;; GPR store GPR load GPR move GPR li GPR lis GPR #
;; FPR store FPR load FPR move AVX store AVX store AVX load
;; AVX load VSX move P9 0 P9 -1 AVX 0/-1 VSX 0
;; VSX -1 P9 const AVX const From SPR To SPR
SPR<->SPR
;; VSX->GPR GPR->VSX
(define_insn "*movdi_internal64"
[(set (match_operand:DI 0 "nonimmediate_operand"
"=YZ, r, r, r, r, r,
m, ^d, ^d, wY, Z, $v,
$v, ^wa, wa, wa, v, wa,
wa, v, v, r, *h, *h,
?r, ?wa")
(match_operand:DI 1 "input_operand"
"r, YZ, r, I, L, nF,
^d, m, ^d, ^v, $v, wY,
Z, ^wa, Oj, wM, OjwM, Oj,
wM, wS, wB, *h, r, 0,
wa, r"))]
"TARGET_POWERPC64
&& (gpc_reg_operand (operands[0], DImode)
|| gpc_reg_operand (operands[1], DImode))"
"@
std%U0%X0 %1,%0
ld%U1%X1 %0,%1
mr %0,%1
li %0,%1
lis %0,%v1
#
stfd%U0%X0 %1,%0
lfd%U1%X1 %0,%1
fmr %0,%1
stxsd %1,%0
stxsdx %x1,%y0
lxsd %0,%1
lxsdx %x0,%y1
xxlor %x0,%x1,%x1
xxspltib %x0,0
xxspltib %x0,255
#
xxlxor %x0,%x0,%x0
xxlorc %x0,%x0,%x0
#
#
mf%1 %0
mt%0 %1
nop
mfvsrd %0,%x1
mtvsrd %x0,%1"
[(set_attr "type"
"store, load, *, *, *, *,
fpstore, fpload, fpsimple, fpstore, fpstore, fpload,
fpload, veclogical, vecsimple, vecsimple, vecsimple,
veclogical,
veclogical, vecsimple, vecsimple, mfjmpr, mtjmpr, *,
mftgpr, mffgpr")
(set_attr "size" "64")
(set_attr "length"
"*, *, *, *, *, 20,
*, *, *, *, *, *,
*, *, *, *, *, *,
*, 8, *, *, *, *,
*, *")
(set_attr "isa"
"*, *, *, *, *, *,
*, *, *, p9v, p7v, p9v,
p7v, *, p9v, p9v, p7v, *,
*, p7v, p7v, *, *, *,
p8v, p8v")])
If I just change the columns, in order to be able to correlate which constraint
goes with which attribute, it would need to be modified to something like:
;; GPR store GPR load GPR move
;; GPR li GPR lis GPR pli GPR #
;; FPR store FPR load FPR move
;; AVX store AVX store AVX load AVX load VSX move
;; P9 0 P9 -1 AVX 0/-1 VSX 0 VSX -1
;; P9 const AVX const
;; From SPR To SPR SPR<->SPR
;; VSX->GPR GPR->VSX
(define_insn "*movdi_internal64"
[(set (match_operand:DI 0 "nonimmediate_operand"
"=YZ, r, r,
r, r, r, r,
m, ^d, ^d,
wY, Z, $v, $v, ^wa,
wa, wa, v, wa, wa,
v, v,
r, *h, *h,
?r, ?wa")
(match_operand:DI 1 "input_operand"
"r, YZ, r,
I, L, eI, nF,
^d, m, ^d, ^v, $v,
wY, Z, ^wa, Oj, wM,
OjwM, Oj, wM, wS, wB,
*h, r, 0,
wa, r"))]
"TARGET_POWERPC64
&& (gpc_reg_operand (operands[0], DImode)
|| gpc_reg_operand (operands[1], DImode))"
"@
std%U0%X0 %1,%0
ld%U1%X1 %0,%1
mr %0,%1
li %0,%1
lis %0,%v1
li %0,%1
#
stfd%U0%X0 %1,%0
lfd%U1%X1 %0,%1
fmr %0,%1
stxsd %1,%0
stxsdx %x1,%y0
lxsd %0,%1
lxsdx %x0,%y1
xxlor %x0,%x1,%x1
xxspltib %x0,0
xxspltib %x0,255
#
xxlxor %x0,%x0,%x0
xxlorc %x0,%x0,%x0
#
#
mf%1 %0
mt%0 %1
nop
mfvsrd %0,%x1
mtvsrd %x0,%1"
[(set_attr "type"
"store, load, *,
*, *, *, *,
fpstore, fpload, fpsimple,
fpstore, fpstore, fpload, fpload, veclogical,
vecsimple, vecsimple, vecsimple, veclogical, veclogical,
vecsimple, vecsimple,
mfjmpr, mtjmpr, *,
mftgpr, mffgpr")
(set_attr "size" "64")
(set_attr "num_insns"
"*, *, *,
*, *, *, 5,
*, *, *,
*, *, *, *, *,
*, *, *, *, *,
2, *,
*, *, *,
*, *")
(set_attr "isa"
"*, *, *,
*, *, fut, *,
*, *, *,
p9v, p7v, p9v, p7v, *,
p9v, p9v, p7v, *, *,
p7v, p7v,
*, *, *,
p8v, p8v")])
Is this what you want?
Now, this is without moving any of the alternatives around. Logically, you
could move gpr/fpr/vsx moves to be together (and maybe the direct moves also).
But I dunno whether this is getting into bike shedding.
> > ; Some DImode loads are best done as a load of -1 followed by a mask
> > -; instruction.
> > +; instruction. On systems that support the PADDI (PLI) instruction,
> > +; num_insns_constant returns 1, so these splitter would not be used for
> > things
> > +; that be loaded with PLI.
>
> That comment doesn't add much at all? This splitter isn't used for
> constants we can load in one insn, that's right. That happily works
> just fine if we have prefixed insns as well.
Well you had asked for the comment many months ago.
--
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
