Skip to content

Commit ddf807f

Browse files
mundaymmundaym
committed
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. Fixes #18906. Change-Id: I95785dcadba03f7e3e94524677e7d8d3d3b9b737 Reviewed-on: https://go-review.googlesource.com/36256 Run-TryBot: Michael Munday <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Cherry Zhang <[email protected]>
1 parent 178307c commit ddf807f

File tree

6 files changed

+297
-202
lines changed

6 files changed

+297
-202
lines changed

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

+6-1
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

+58-51
Original file line numberDiff line numberDiff line change
@@ -309,9 +309,12 @@
309309

310310
// Lowering loads
311311
(Load <t> ptr mem) && (is64BitInt(t) || isPtr(t)) -> (MOVDload ptr mem)
312-
(Load <t> ptr mem) && is32BitInt(t) -> (MOVWZload ptr mem)
313-
(Load <t> ptr mem) && is16BitInt(t) -> (MOVHZload ptr mem)
314-
(Load <t> ptr mem) && (t.IsBoolean() || is8BitInt(t)) -> (MOVBZload ptr mem)
312+
(Load <t> ptr mem) && is32BitInt(t) && isSigned(t) -> (MOVWload ptr mem)
313+
(Load <t> ptr mem) && is32BitInt(t) && !isSigned(t) -> (MOVWZload ptr mem)
314+
(Load <t> ptr mem) && is16BitInt(t) && isSigned(t) -> (MOVHload ptr mem)
315+
(Load <t> ptr mem) && is16BitInt(t) && !isSigned(t) -> (MOVHZload ptr mem)
316+
(Load <t> ptr mem) && is8BitInt(t) && isSigned(t) -> (MOVBload ptr mem)
317+
(Load <t> ptr mem) && (t.IsBoolean() || (is8BitInt(t) && !isSigned(t))) -> (MOVBZload ptr mem)
315318
(Load <t> ptr mem) && is32BitFloat(t) -> (FMOVSload ptr mem)
316319
(Load <t> ptr mem) && is64BitFloat(t) -> (FMOVDload ptr mem)
317320

@@ -442,16 +445,20 @@
442445
// ***************************
443446
// TODO: Should the optimizations be a separate pass?
444447

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

456463
// Fold boolean tests into blocks.
457464
(NE (CMPWconst [0] (MOVDLT (MOVDconst [0]) (MOVDconst [1]) cmp)) yes no) -> (LT cmp yes no)
@@ -584,46 +591,46 @@
584591
(MOVDNE x y (InvertFlags cmp)) -> (MOVDNE x y cmp)
585592

586593
// don't extend after proper load
587-
(MOVBreg x:(MOVBload _ _)) -> x
588-
(MOVBZreg x:(MOVBZload _ _)) -> x
589-
(MOVHreg x:(MOVBload _ _)) -> x
590-
(MOVHreg x:(MOVBZload _ _)) -> x
591-
(MOVHreg x:(MOVHload _ _)) -> x
592-
(MOVHZreg x:(MOVBZload _ _)) -> x
593-
(MOVHZreg x:(MOVHZload _ _)) -> x
594-
(MOVWreg x:(MOVBload _ _)) -> x
595-
(MOVWreg x:(MOVBZload _ _)) -> x
596-
(MOVWreg x:(MOVHload _ _)) -> x
597-
(MOVWreg x:(MOVHZload _ _)) -> x
598-
(MOVWreg x:(MOVWload _ _)) -> x
599-
(MOVWZreg x:(MOVBZload _ _)) -> x
600-
(MOVWZreg x:(MOVHZload _ _)) -> x
601-
(MOVWZreg x:(MOVWZload _ _)) -> x
594+
(MOVBreg x:(MOVBload _ _)) -> (MOVDreg x)
595+
(MOVBZreg x:(MOVBZload _ _)) -> (MOVDreg x)
596+
(MOVHreg x:(MOVBload _ _)) -> (MOVDreg x)
597+
(MOVHreg x:(MOVBZload _ _)) -> (MOVDreg x)
598+
(MOVHreg x:(MOVHload _ _)) -> (MOVDreg x)
599+
(MOVHZreg x:(MOVBZload _ _)) -> (MOVDreg x)
600+
(MOVHZreg x:(MOVHZload _ _)) -> (MOVDreg x)
601+
(MOVWreg x:(MOVBload _ _)) -> (MOVDreg x)
602+
(MOVWreg x:(MOVBZload _ _)) -> (MOVDreg x)
603+
(MOVWreg x:(MOVHload _ _)) -> (MOVDreg x)
604+
(MOVWreg x:(MOVHZload _ _)) -> (MOVDreg x)
605+
(MOVWreg x:(MOVWload _ _)) -> (MOVDreg x)
606+
(MOVWZreg x:(MOVBZload _ _)) -> (MOVDreg x)
607+
(MOVWZreg x:(MOVHZload _ _)) -> (MOVDreg x)
608+
(MOVWZreg x:(MOVWZload _ _)) -> (MOVDreg x)
602609

603610
// don't extend if argument is already extended
604-
(MOVBreg x:(Arg <t>)) && is8BitInt(t) && isSigned(t) -> x
605-
(MOVBZreg x:(Arg <t>)) && is8BitInt(t) && !isSigned(t) -> x
606-
(MOVHreg x:(Arg <t>)) && (is8BitInt(t) || is16BitInt(t)) && isSigned(t) -> x
607-
(MOVHZreg x:(Arg <t>)) && (is8BitInt(t) || is16BitInt(t)) && !isSigned(t) -> x
608-
(MOVWreg x:(Arg <t>)) && (is8BitInt(t) || is16BitInt(t) || is32BitInt(t)) && isSigned(t) -> x
609-
(MOVWZreg x:(Arg <t>)) && (is8BitInt(t) || is16BitInt(t) || is32BitInt(t)) && !isSigned(t) -> x
611+
(MOVBreg x:(Arg <t>)) && is8BitInt(t) && isSigned(t) -> (MOVDreg x)
612+
(MOVBZreg x:(Arg <t>)) && is8BitInt(t) && !isSigned(t) -> (MOVDreg x)
613+
(MOVHreg x:(Arg <t>)) && (is8BitInt(t) || is16BitInt(t)) && isSigned(t) -> (MOVDreg x)
614+
(MOVHZreg x:(Arg <t>)) && (is8BitInt(t) || is16BitInt(t)) && !isSigned(t) -> (MOVDreg x)
615+
(MOVWreg x:(Arg <t>)) && (is8BitInt(t) || is16BitInt(t) || is32BitInt(t)) && isSigned(t) -> (MOVDreg x)
616+
(MOVWZreg x:(Arg <t>)) && (is8BitInt(t) || is16BitInt(t) || is32BitInt(t)) && !isSigned(t) -> (MOVDreg x)
610617

611618
// fold double extensions
612-
(MOVBreg x:(MOVBreg _)) -> x
613-
(MOVBZreg x:(MOVBZreg _)) -> x
614-
(MOVHreg x:(MOVBreg _)) -> x
615-
(MOVHreg x:(MOVBZreg _)) -> x
616-
(MOVHreg x:(MOVHreg _)) -> x
617-
(MOVHZreg x:(MOVBZreg _)) -> x
618-
(MOVHZreg x:(MOVHZreg _)) -> x
619-
(MOVWreg x:(MOVBreg _)) -> x
620-
(MOVWreg x:(MOVBZreg _)) -> x
621-
(MOVWreg x:(MOVHreg _)) -> x
622-
(MOVWreg x:(MOVHreg _)) -> x
623-
(MOVWreg x:(MOVWreg _)) -> x
624-
(MOVWZreg x:(MOVBZreg _)) -> x
625-
(MOVWZreg x:(MOVHZreg _)) -> x
626-
(MOVWZreg x:(MOVWZreg _)) -> x
619+
(MOVBreg x:(MOVBreg _)) -> (MOVDreg x)
620+
(MOVBZreg x:(MOVBZreg _)) -> (MOVDreg x)
621+
(MOVHreg x:(MOVBreg _)) -> (MOVDreg x)
622+
(MOVHreg x:(MOVBZreg _)) -> (MOVDreg x)
623+
(MOVHreg x:(MOVHreg _)) -> (MOVDreg x)
624+
(MOVHZreg x:(MOVBZreg _)) -> (MOVDreg x)
625+
(MOVHZreg x:(MOVHZreg _)) -> (MOVDreg x)
626+
(MOVWreg x:(MOVBreg _)) -> (MOVDreg x)
627+
(MOVWreg x:(MOVBZreg _)) -> (MOVDreg x)
628+
(MOVWreg x:(MOVHreg _)) -> (MOVDreg x)
629+
(MOVWreg x:(MOVHreg _)) -> (MOVDreg x)
630+
(MOVWreg x:(MOVWreg _)) -> (MOVDreg x)
631+
(MOVWZreg x:(MOVBZreg _)) -> (MOVDreg x)
632+
(MOVWZreg x:(MOVHZreg _)) -> (MOVDreg x)
633+
(MOVWZreg x:(MOVWZreg _)) -> (MOVDreg x)
627634

628635
// fold extensions into constants
629636
(MOVBreg (MOVDconst [c])) -> (MOVDconst [int64(int8(c))])
@@ -653,10 +660,10 @@
653660
(MOVWZreg x:(MOVWZloadidx [off] {sym} ptr idx mem)) && x.Uses == 1 && clobber(x) -> @x.Block (MOVWZloadidx <v.Type> [off] {sym} ptr idx mem)
654661

655662
// replace load from same location as preceding store with copy
656-
(MOVBZload [off] {sym} ptr (MOVBstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x
657-
(MOVHZload [off] {sym} ptr (MOVHstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x
658-
(MOVWZload [off] {sym} ptr (MOVWstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x
659-
(MOVDload [off] {sym} ptr (MOVDstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x
663+
(MOVBZload [off] {sym} ptr (MOVBstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> (MOVDreg x)
664+
(MOVHZload [off] {sym} ptr (MOVHstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> (MOVDreg x)
665+
(MOVWZload [off] {sym} ptr (MOVWstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> (MOVDreg x)
666+
(MOVDload [off] {sym} ptr (MOVDstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> (MOVDreg x)
660667

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

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

+3
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

+28
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
@@ -18578,6 +18580,32 @@ var opcodeTable = [...]opInfo{
1857818580
},
1857918581
},
1858018582
},
18583+
{
18584+
name: "MOVDreg",
18585+
argLen: 1,
18586+
asm: s390x.AMOVD,
18587+
reg: regInfo{
18588+
inputs: []inputInfo{
18589+
{0, 54271}, // R0 R1 R2 R3 R4 R5 R6 R7 R8 R9 R12 R14 SP
18590+
},
18591+
outputs: []outputInfo{
18592+
{0, 21503}, // R0 R1 R2 R3 R4 R5 R6 R7 R8 R9 R12 R14
18593+
},
18594+
},
18595+
},
18596+
{
18597+
name: "MOVDnop",
18598+
argLen: 1,
18599+
resultInArg0: true,
18600+
reg: regInfo{
18601+
inputs: []inputInfo{
18602+
{0, 21503}, // R0 R1 R2 R3 R4 R5 R6 R7 R8 R9 R12 R14
18603+
},
18604+
outputs: []outputInfo{
18605+
{0, 21503}, // R0 R1 R2 R3 R4 R5 R6 R7 R8 R9 R12 R14
18606+
},
18607+
},
18608+
},
1858118609
{
1858218610
name: "MOVDconst",
1858318611
auxType: auxInt64,

0 commit comments

Comments
 (0)