Skip to content

Commit 758a728

Browse files
committed
[release-branch.go1.8] cmd/compile: fix type propagation through s390x SSA rules
This CL fixes two issues: 1. Load ops were initially always lowered to unsigned loads, even for signed types. This was fine by itself however LoadReg ops (used to re-load spilled values) were lowered to signed loads for signed types. This meant that spills could invalidate optimizations that assumed the original unsigned load. 2. Types were not always being maintained correctly through rules designed to eliminate unnecessary zero and sign extensions. Updates #18906 and fixes #18958 (backport of CL 36256 to 1.8). Change-Id: Id44953b0f644cad047e8474edbd24e8a344ca9a7 Reviewed-on: https://go-review.googlesource.com/36350 Reviewed-by: Brad Fitzpatrick <[email protected]>
1 parent 4707045 commit 758a728

File tree

6 files changed

+297
-202
lines changed

6 files changed

+297
-202
lines changed

src/cmd/compile/internal/s390x/ssa.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ func ssaGenValue(s *gc.SSAGenState, v *ssa.Value) {
424424
p.To.Type = obj.TYPE_MEM
425425
p.To.Reg = v.Args[0].Reg()
426426
gc.AddAux2(&p.To, v, sc.Off())
427-
case ssa.OpCopy, ssa.OpS390XMOVDconvert:
427+
case ssa.OpCopy, ssa.OpS390XMOVDconvert, ssa.OpS390XMOVDreg:
428428
if v.Type.IsMemory() {
429429
return
430430
}
@@ -433,6 +433,11 @@ func ssaGenValue(s *gc.SSAGenState, v *ssa.Value) {
433433
if x != y {
434434
opregreg(moveByType(v.Type), y, x)
435435
}
436+
case ssa.OpS390XMOVDnop:
437+
if v.Reg() != v.Args[0].Reg() {
438+
v.Fatalf("input[0] and output not in same register %s", v.LongString())
439+
}
440+
// nothing to do
436441
case ssa.OpLoadReg:
437442
if v.Type.IsFlags() {
438443
v.Fatalf("load flags not implemented: %v", v.LongString())

src/cmd/compile/internal/ssa/gen/S390X.rules

Lines changed: 58 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -312,9 +312,12 @@
312312

313313
// Lowering loads
314314
(Load <t> ptr mem) && (is64BitInt(t) || isPtr(t)) -> (MOVDload ptr mem)
315-
(Load <t> ptr mem) && is32BitInt(t) -> (MOVWZload ptr mem)
316-
(Load <t> ptr mem) && is16BitInt(t) -> (MOVHZload ptr mem)
317-
(Load <t> ptr mem) && (t.IsBoolean() || is8BitInt(t)) -> (MOVBZload ptr mem)
315+
(Load <t> ptr mem) && is32BitInt(t) && isSigned(t) -> (MOVWload ptr mem)
316+
(Load <t> ptr mem) && is32BitInt(t) && !isSigned(t) -> (MOVWZload ptr mem)
317+
(Load <t> ptr mem) && is16BitInt(t) && isSigned(t) -> (MOVHload ptr mem)
318+
(Load <t> ptr mem) && is16BitInt(t) && !isSigned(t) -> (MOVHZload ptr mem)
319+
(Load <t> ptr mem) && is8BitInt(t) && isSigned(t) -> (MOVBload ptr mem)
320+
(Load <t> ptr mem) && (t.IsBoolean() || (is8BitInt(t) && !isSigned(t))) -> (MOVBZload ptr mem)
318321
(Load <t> ptr mem) && is32BitFloat(t) -> (FMOVSload ptr mem)
319322
(Load <t> ptr mem) && is64BitFloat(t) -> (FMOVDload ptr mem)
320323

@@ -445,16 +448,20 @@
445448
// ***************************
446449
// TODO: Should the optimizations be a separate pass?
447450

451+
// if a register move has only 1 use, just use the same register without emitting instruction
452+
// MOVDnop doesn't emit instruction, only for ensuring the type.
453+
(MOVDreg x) && x.Uses == 1 -> (MOVDnop x)
454+
448455
// Fold sign extensions into conditional moves of constants.
449456
// Designed to remove the MOVBZreg inserted by the If lowering.
450-
(MOVBZreg x:(MOVDLT (MOVDconst [c]) (MOVDconst [d]) _)) && int64(uint8(c)) == c && int64(uint8(d)) == d -> x
451-
(MOVBZreg x:(MOVDLE (MOVDconst [c]) (MOVDconst [d]) _)) && int64(uint8(c)) == c && int64(uint8(d)) == d -> x
452-
(MOVBZreg x:(MOVDGT (MOVDconst [c]) (MOVDconst [d]) _)) && int64(uint8(c)) == c && int64(uint8(d)) == d -> x
453-
(MOVBZreg x:(MOVDGE (MOVDconst [c]) (MOVDconst [d]) _)) && int64(uint8(c)) == c && int64(uint8(d)) == d -> x
454-
(MOVBZreg x:(MOVDEQ (MOVDconst [c]) (MOVDconst [d]) _)) && int64(uint8(c)) == c && int64(uint8(d)) == d -> x
455-
(MOVBZreg x:(MOVDNE (MOVDconst [c]) (MOVDconst [d]) _)) && int64(uint8(c)) == c && int64(uint8(d)) == d -> x
456-
(MOVBZreg x:(MOVDGTnoinv (MOVDconst [c]) (MOVDconst [d]) _)) && int64(uint8(c)) == c && int64(uint8(d)) == d -> x
457-
(MOVBZreg x:(MOVDGEnoinv (MOVDconst [c]) (MOVDconst [d]) _)) && int64(uint8(c)) == c && int64(uint8(d)) == d -> x
457+
(MOVBZreg x:(MOVDLT (MOVDconst [c]) (MOVDconst [d]) _)) && int64(uint8(c)) == c && int64(uint8(d)) == d -> (MOVDreg x)
458+
(MOVBZreg x:(MOVDLE (MOVDconst [c]) (MOVDconst [d]) _)) && int64(uint8(c)) == c && int64(uint8(d)) == d -> (MOVDreg x)
459+
(MOVBZreg x:(MOVDGT (MOVDconst [c]) (MOVDconst [d]) _)) && int64(uint8(c)) == c && int64(uint8(d)) == d -> (MOVDreg x)
460+
(MOVBZreg x:(MOVDGE (MOVDconst [c]) (MOVDconst [d]) _)) && int64(uint8(c)) == c && int64(uint8(d)) == d -> (MOVDreg x)
461+
(MOVBZreg x:(MOVDEQ (MOVDconst [c]) (MOVDconst [d]) _)) && int64(uint8(c)) == c && int64(uint8(d)) == d -> (MOVDreg x)
462+
(MOVBZreg x:(MOVDNE (MOVDconst [c]) (MOVDconst [d]) _)) && int64(uint8(c)) == c && int64(uint8(d)) == d -> (MOVDreg x)
463+
(MOVBZreg x:(MOVDGTnoinv (MOVDconst [c]) (MOVDconst [d]) _)) && int64(uint8(c)) == c && int64(uint8(d)) == d -> (MOVDreg x)
464+
(MOVBZreg x:(MOVDGEnoinv (MOVDconst [c]) (MOVDconst [d]) _)) && int64(uint8(c)) == c && int64(uint8(d)) == d -> (MOVDreg x)
458465

459466
// Fold boolean tests into blocks.
460467
(NE (CMPWconst [0] (MOVDLT (MOVDconst [0]) (MOVDconst [1]) cmp)) yes no) -> (LT cmp yes no)
@@ -572,46 +579,46 @@
572579
(MOVDNE x y (InvertFlags cmp)) -> (MOVDNE x y cmp)
573580

574581
// don't extend after proper load
575-
(MOVBreg x:(MOVBload _ _)) -> x
576-
(MOVBZreg x:(MOVBZload _ _)) -> x
577-
(MOVHreg x:(MOVBload _ _)) -> x
578-
(MOVHreg x:(MOVBZload _ _)) -> x
579-
(MOVHreg x:(MOVHload _ _)) -> x
580-
(MOVHZreg x:(MOVBZload _ _)) -> x
581-
(MOVHZreg x:(MOVHZload _ _)) -> x
582-
(MOVWreg x:(MOVBload _ _)) -> x
583-
(MOVWreg x:(MOVBZload _ _)) -> x
584-
(MOVWreg x:(MOVHload _ _)) -> x
585-
(MOVWreg x:(MOVHZload _ _)) -> x
586-
(MOVWreg x:(MOVWload _ _)) -> x
587-
(MOVWZreg x:(MOVBZload _ _)) -> x
588-
(MOVWZreg x:(MOVHZload _ _)) -> x
589-
(MOVWZreg x:(MOVWZload _ _)) -> x
582+
(MOVBreg x:(MOVBload _ _)) -> (MOVDreg x)
583+
(MOVBZreg x:(MOVBZload _ _)) -> (MOVDreg x)
584+
(MOVHreg x:(MOVBload _ _)) -> (MOVDreg x)
585+
(MOVHreg x:(MOVBZload _ _)) -> (MOVDreg x)
586+
(MOVHreg x:(MOVHload _ _)) -> (MOVDreg x)
587+
(MOVHZreg x:(MOVBZload _ _)) -> (MOVDreg x)
588+
(MOVHZreg x:(MOVHZload _ _)) -> (MOVDreg x)
589+
(MOVWreg x:(MOVBload _ _)) -> (MOVDreg x)
590+
(MOVWreg x:(MOVBZload _ _)) -> (MOVDreg x)
591+
(MOVWreg x:(MOVHload _ _)) -> (MOVDreg x)
592+
(MOVWreg x:(MOVHZload _ _)) -> (MOVDreg x)
593+
(MOVWreg x:(MOVWload _ _)) -> (MOVDreg x)
594+
(MOVWZreg x:(MOVBZload _ _)) -> (MOVDreg x)
595+
(MOVWZreg x:(MOVHZload _ _)) -> (MOVDreg x)
596+
(MOVWZreg x:(MOVWZload _ _)) -> (MOVDreg x)
590597

591598
// don't extend if argument is already extended
592-
(MOVBreg x:(Arg <t>)) && is8BitInt(t) && isSigned(t) -> x
593-
(MOVBZreg x:(Arg <t>)) && is8BitInt(t) && !isSigned(t) -> x
594-
(MOVHreg x:(Arg <t>)) && (is8BitInt(t) || is16BitInt(t)) && isSigned(t) -> x
595-
(MOVHZreg x:(Arg <t>)) && (is8BitInt(t) || is16BitInt(t)) && !isSigned(t) -> x
596-
(MOVWreg x:(Arg <t>)) && (is8BitInt(t) || is16BitInt(t) || is32BitInt(t)) && isSigned(t) -> x
597-
(MOVWZreg x:(Arg <t>)) && (is8BitInt(t) || is16BitInt(t) || is32BitInt(t)) && !isSigned(t) -> x
599+
(MOVBreg x:(Arg <t>)) && is8BitInt(t) && isSigned(t) -> (MOVDreg x)
600+
(MOVBZreg x:(Arg <t>)) && is8BitInt(t) && !isSigned(t) -> (MOVDreg x)
601+
(MOVHreg x:(Arg <t>)) && (is8BitInt(t) || is16BitInt(t)) && isSigned(t) -> (MOVDreg x)
602+
(MOVHZreg x:(Arg <t>)) && (is8BitInt(t) || is16BitInt(t)) && !isSigned(t) -> (MOVDreg x)
603+
(MOVWreg x:(Arg <t>)) && (is8BitInt(t) || is16BitInt(t) || is32BitInt(t)) && isSigned(t) -> (MOVDreg x)
604+
(MOVWZreg x:(Arg <t>)) && (is8BitInt(t) || is16BitInt(t) || is32BitInt(t)) && !isSigned(t) -> (MOVDreg x)
598605

599606
// fold double extensions
600-
(MOVBreg x:(MOVBreg _)) -> x
601-
(MOVBZreg x:(MOVBZreg _)) -> x
602-
(MOVHreg x:(MOVBreg _)) -> x
603-
(MOVHreg x:(MOVBZreg _)) -> x
604-
(MOVHreg x:(MOVHreg _)) -> x
605-
(MOVHZreg x:(MOVBZreg _)) -> x
606-
(MOVHZreg x:(MOVHZreg _)) -> x
607-
(MOVWreg x:(MOVBreg _)) -> x
608-
(MOVWreg x:(MOVBZreg _)) -> x
609-
(MOVWreg x:(MOVHreg _)) -> x
610-
(MOVWreg x:(MOVHreg _)) -> x
611-
(MOVWreg x:(MOVWreg _)) -> x
612-
(MOVWZreg x:(MOVBZreg _)) -> x
613-
(MOVWZreg x:(MOVHZreg _)) -> x
614-
(MOVWZreg x:(MOVWZreg _)) -> x
607+
(MOVBreg x:(MOVBreg _)) -> (MOVDreg x)
608+
(MOVBZreg x:(MOVBZreg _)) -> (MOVDreg x)
609+
(MOVHreg x:(MOVBreg _)) -> (MOVDreg x)
610+
(MOVHreg x:(MOVBZreg _)) -> (MOVDreg x)
611+
(MOVHreg x:(MOVHreg _)) -> (MOVDreg x)
612+
(MOVHZreg x:(MOVBZreg _)) -> (MOVDreg x)
613+
(MOVHZreg x:(MOVHZreg _)) -> (MOVDreg x)
614+
(MOVWreg x:(MOVBreg _)) -> (MOVDreg x)
615+
(MOVWreg x:(MOVBZreg _)) -> (MOVDreg x)
616+
(MOVWreg x:(MOVHreg _)) -> (MOVDreg x)
617+
(MOVWreg x:(MOVHreg _)) -> (MOVDreg x)
618+
(MOVWreg x:(MOVWreg _)) -> (MOVDreg x)
619+
(MOVWZreg x:(MOVBZreg _)) -> (MOVDreg x)
620+
(MOVWZreg x:(MOVHZreg _)) -> (MOVDreg x)
621+
(MOVWZreg x:(MOVWZreg _)) -> (MOVDreg x)
615622

616623
// fold extensions into constants
617624
(MOVBreg (MOVDconst [c])) -> (MOVDconst [int64(int8(c))])
@@ -641,10 +648,10 @@
641648
(MOVWZreg x:(MOVWZloadidx [off] {sym} ptr idx mem)) && x.Uses == 1 && clobber(x) -> @x.Block (MOVWZloadidx <v.Type> [off] {sym} ptr idx mem)
642649

643650
// replace load from same location as preceding store with copy
644-
(MOVBZload [off] {sym} ptr (MOVBstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x
645-
(MOVHZload [off] {sym} ptr (MOVHstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x
646-
(MOVWZload [off] {sym} ptr (MOVWstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x
647-
(MOVDload [off] {sym} ptr (MOVDstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x
651+
(MOVBZload [off] {sym} ptr (MOVBstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> (MOVDreg x)
652+
(MOVHZload [off] {sym} ptr (MOVHstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> (MOVDreg x)
653+
(MOVWZload [off] {sym} ptr (MOVWstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> (MOVDreg x)
654+
(MOVDload [off] {sym} ptr (MOVDstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> (MOVDreg x)
648655

649656
// Don't extend before storing
650657
(MOVWstore [off] {sym} ptr (MOVWreg x) mem) -> (MOVWstore [off] {sym} ptr x mem)

src/cmd/compile/internal/ssa/gen/S390XOps.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,9 @@ func init() {
311311
{name: "MOVHZreg", argLength: 1, reg: gp11sp, asm: "MOVHZ", typ: "UInt64"}, // zero extend arg0 from int16 to int64
312312
{name: "MOVWreg", argLength: 1, reg: gp11sp, asm: "MOVW", typ: "Int64"}, // sign extend arg0 from int32 to int64
313313
{name: "MOVWZreg", argLength: 1, reg: gp11sp, asm: "MOVWZ", typ: "UInt64"}, // zero extend arg0 from int32 to int64
314+
{name: "MOVDreg", argLength: 1, reg: gp11sp, asm: "MOVD"}, // move from arg0
315+
316+
{name: "MOVDnop", argLength: 1, reg: gp11, resultInArg0: true}, // nop, return arg0 in same register
314317

315318
{name: "MOVDconst", reg: gp01, asm: "MOVD", typ: "UInt64", aux: "Int64", rematerializeable: true}, // auxint
316319

src/cmd/compile/internal/ssa/opGen.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1473,6 +1473,8 @@ const (
14731473
OpS390XMOVHZreg
14741474
OpS390XMOVWreg
14751475
OpS390XMOVWZreg
1476+
OpS390XMOVDreg
1477+
OpS390XMOVDnop
14761478
OpS390XMOVDconst
14771479
OpS390XCFDBRA
14781480
OpS390XCGDBRA
@@ -18570,6 +18572,32 @@ var opcodeTable = [...]opInfo{
1857018572
},
1857118573
},
1857218574
},
18575+
{
18576+
name: "MOVDreg",
18577+
argLen: 1,
18578+
asm: s390x.AMOVD,
18579+
reg: regInfo{
18580+
inputs: []inputInfo{
18581+
{0, 54271}, // R0 R1 R2 R3 R4 R5 R6 R7 R8 R9 R12 R14 SP
18582+
},
18583+
outputs: []outputInfo{
18584+
{0, 21503}, // R0 R1 R2 R3 R4 R5 R6 R7 R8 R9 R12 R14
18585+
},
18586+
},
18587+
},
18588+
{
18589+
name: "MOVDnop",
18590+
argLen: 1,
18591+
resultInArg0: true,
18592+
reg: regInfo{
18593+
inputs: []inputInfo{
18594+
{0, 21503}, // R0 R1 R2 R3 R4 R5 R6 R7 R8 R9 R12 R14
18595+
},
18596+
outputs: []outputInfo{
18597+
{0, 21503}, // R0 R1 R2 R3 R4 R5 R6 R7 R8 R9 R12 R14
18598+
},
18599+
},
18600+
},
1857318601
{
1857418602
name: "MOVDconst",
1857518603
auxType: auxInt64,

0 commit comments

Comments
 (0)