Skip to content

Commit

Permalink
cmd/compile: don't lower OpConvert
Browse files Browse the repository at this point in the history
Currently, each architecture lowers OpConvert to an arch-specific
OpXXXconvert. This is silly because OpConvert means the same thing on
all architectures and is logically a no-op that exists only to keep
track of conversions to and from unsafe.Pointer. Furthermore, lowering
it makes it harder to recognize in other analyses, particularly
liveness analysis.

This CL eliminates the lowering of OpConvert, leaving it as the
generic op until code generation time.

The main complexity here is that we still need to register-allocate
OpConvert operations. Currently, each arch's lowered OpConvert
specifies all GP registers in its register mask. Ideally, OpConvert
wouldn't affect value homing at all, and we could just copy the home
of OpConvert's source, but this can potentially home an OpConvert in a
LocalSlot, which neither regalloc nor stackalloc expect. Rather than
try to disentangle this assumption from regalloc and stackalloc, we
continue to register-allocate OpConvert, but teach regalloc that
OpConvert can be allocated to any allocatable GP register.

For #24543.

Change-Id: I795a6aee5fd94d4444a7bafac3838a400c9f7bb6
Reviewed-on: https://go-review.googlesource.com/108496
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
  • Loading branch information
aclements committed Apr 20, 2018
1 parent c1a466b commit 8871c93
Show file tree
Hide file tree
Showing 38 changed files with 44 additions and 381 deletions.
4 changes: 0 additions & 4 deletions src/cmd/compile/internal/amd64/ssa.go
Original file line number Diff line number Diff line change
Expand Up @@ -846,10 +846,6 @@ func ssaGenValue(s *gc.SSAGenState, v *ssa.Value) {
p.To.Sym = gc.Duffcopy
p.To.Offset = v.AuxInt

case ssa.OpAMD64MOVQconvert, ssa.OpAMD64MOVLconvert:
if v.Args[0].Reg() != v.Reg() {
v.Fatalf("MOVXconvert should be a no-op")
}
case ssa.OpCopy: // TODO: use MOVQreg for reg->reg copies instead of OpCopy?
if v.Type.IsMemory() {
return
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/arm/ssa.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func genregshift(s *gc.SSAGenState, as obj.As, r0, r1, r2, r int16, typ int64) *

func ssaGenValue(s *gc.SSAGenState, v *ssa.Value) {
switch v.Op {
case ssa.OpCopy, ssa.OpARMMOVWconvert, ssa.OpARMMOVWreg:
case ssa.OpCopy, ssa.OpARMMOVWreg:
if v.Type.IsMemory() {
return
}
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/arm64/ssa.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func genshift(s *gc.SSAGenState, as obj.As, r0, r1, r int16, typ int64, n int64)

func ssaGenValue(s *gc.SSAGenState, v *ssa.Value) {
switch v.Op {
case ssa.OpCopy, ssa.OpARM64MOVDconvert, ssa.OpARM64MOVDreg:
case ssa.OpCopy, ssa.OpARM64MOVDreg:
if v.Type.IsMemory() {
return
}
Expand Down
5 changes: 5 additions & 0 deletions src/cmd/compile/internal/gc/ssa.go
Original file line number Diff line number Diff line change
Expand Up @@ -4806,6 +4806,11 @@ func genssa(f *ssa.Func, pp *Progs) {
}
case ssa.OpPhi:
CheckLoweredPhi(v)
case ssa.OpConvert:
// nothing to do; no-op conversion for liveness
if v.Args[0].Reg() != v.Reg() {
v.Fatalf("OpConvert should be a no-op: %s; %s", v.Args[0].LongString(), v.LongString())
}
default:
// let the backend handle it
// Special case for first line in function; move it to the start.
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/mips/ssa.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func storeByType(t *types.Type, r int16) obj.As {

func ssaGenValue(s *gc.SSAGenState, v *ssa.Value) {
switch v.Op {
case ssa.OpCopy, ssa.OpMIPSMOVWconvert, ssa.OpMIPSMOVWreg:
case ssa.OpCopy, ssa.OpMIPSMOVWreg:
t := v.Type
if t.IsMemory() {
return
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/mips64/ssa.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func storeByType(t *types.Type, r int16) obj.As {

func ssaGenValue(s *gc.SSAGenState, v *ssa.Value) {
switch v.Op {
case ssa.OpCopy, ssa.OpMIPS64MOVVconvert, ssa.OpMIPS64MOVVreg:
case ssa.OpCopy, ssa.OpMIPS64MOVVreg:
if v.Type.IsMemory() {
return
}
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/ppc64/ssa.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func ssaGenISEL(s *gc.SSAGenState, v *ssa.Value, cr int64, r1, r2 int16) {

func ssaGenValue(s *gc.SSAGenState, v *ssa.Value) {
switch v.Op {
case ssa.OpCopy, ssa.OpPPC64MOVDconvert:
case ssa.OpCopy:
t := v.Type
if t.IsMemory() {
return
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/s390x/ssa.go
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ func ssaGenValue(s *gc.SSAGenState, v *ssa.Value) {
p.To.Type = obj.TYPE_MEM
p.To.Reg = v.Args[0].Reg()
gc.AddAux2(&p.To, v, sc.Off())
case ssa.OpCopy, ssa.OpS390XMOVDconvert, ssa.OpS390XMOVDreg:
case ssa.OpCopy, ssa.OpS390XMOVDreg:
if v.Type.IsMemory() {
return
}
Expand Down
1 change: 0 additions & 1 deletion src/cmd/compile/internal/ssa/gen/386.rules
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,6 @@
(InterCall [argwid] entry mem) -> (CALLinter [argwid] entry mem)

// Miscellaneous
(Convert <t> x mem) -> (MOVLconvert <t> x mem)
(IsNonNil p) -> (SETNE (TESTL p p))
(IsInBounds idx len) -> (SETB (CMPL idx len))
(IsSliceInBounds idx len) -> (SETBE (CMPL idx len))
Expand Down
7 changes: 0 additions & 7 deletions src/cmd/compile/internal/ssa/gen/386Ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -469,13 +469,6 @@ func init() {
// It saves all GP registers if necessary, but may clobber others.
{name: "LoweredWB", argLength: 3, reg: regInfo{inputs: []regMask{buildReg("DI"), ax}, clobbers: callerSave &^ gp}, clobberFlags: true, aux: "Sym", symEffect: "None"},

// MOVLconvert converts between pointers and integers.
// We have a special op for this so as to not confuse GC
// (particularly stack maps). It takes a memory arg so it
// gets correctly ordered with respect to GC safepoints.
// arg0=ptr/int arg1=mem, output=int/ptr
{name: "MOVLconvert", argLength: 2, reg: gp11, asm: "MOVL", resultInArg0: true, zeroWidth: true},

// Constant flag values. For any comparison, there are 5 possible
// outcomes: the three from the signed total order (<,==,>) and the
// three from the unsigned total order. The == cases overlap.
Expand Down
2 changes: 0 additions & 2 deletions src/cmd/compile/internal/ssa/gen/AMD64.rules
Original file line number Diff line number Diff line change
Expand Up @@ -459,8 +459,6 @@
(CMOV(QEQ|QGT|QGE|QCS|QLS|LEQ|LGT|LGE|LCS|LLS|WEQ|WGT|WGE|WCS|WLS) y _ (FlagLT_UGT)) -> y

// Miscellaneous
(Convert <t> x mem) && config.PtrSize == 8 -> (MOVQconvert <t> x mem)
(Convert <t> x mem) && config.PtrSize == 4 -> (MOVLconvert <t> x mem)
(IsNonNil p) && config.PtrSize == 8 -> (SETNE (TESTQ p p))
(IsNonNil p) && config.PtrSize == 4 -> (SETNE (TESTL p p))
(IsInBounds idx len) && config.PtrSize == 8 -> (SETB (CMPQ idx len))
Expand Down
8 changes: 0 additions & 8 deletions src/cmd/compile/internal/ssa/gen/AMD64Ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -642,14 +642,6 @@ func init() {
// It saves all GP registers if necessary, but may clobber others.
{name: "LoweredWB", argLength: 3, reg: regInfo{inputs: []regMask{buildReg("DI"), ax}, clobbers: callerSave &^ gp}, clobberFlags: true, aux: "Sym", symEffect: "None"},

// MOVQconvert converts between pointers and integers.
// We have a special op for this so as to not confuse GC
// (particularly stack maps). It takes a memory arg so it
// gets correctly ordered with respect to GC safepoints.
// arg0=ptr/int arg1=mem, output=int/ptr
{name: "MOVQconvert", argLength: 2, reg: gp11, asm: "MOVQ", resultInArg0: true, zeroWidth: true},
{name: "MOVLconvert", argLength: 2, reg: gp11, asm: "MOVL", resultInArg0: true, zeroWidth: true}, // amd64p32 equivalent

// Constant flag values. For any comparison, there are 5 possible
// outcomes: the three from the signed total order (<,==,>) and the
// three from the unsigned total order. The == cases overlap.
Expand Down
1 change: 0 additions & 1 deletion src/cmd/compile/internal/ssa/gen/ARM.rules
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,6 @@
// pseudo-ops
(GetClosurePtr) -> (LoweredGetClosurePtr)
(GetCallerSP) -> (LoweredGetCallerSP)
(Convert x mem) -> (MOVWconvert x mem)

// Absorb pseudo-ops into blocks.
(If (Equal cc) yes no) -> (EQ cc yes no)
Expand Down
1 change: 0 additions & 1 deletion src/cmd/compile/internal/ssa/gen/ARM64.rules
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,6 @@
// pseudo-ops
(GetClosurePtr) -> (LoweredGetClosurePtr)
(GetCallerSP) -> (LoweredGetCallerSP)
(Convert x mem) -> (MOVDconvert x mem)

// Absorb pseudo-ops into blocks.
(If (Equal cc) yes no) -> (EQ cc yes no)
Expand Down
7 changes: 0 additions & 7 deletions src/cmd/compile/internal/ssa/gen/ARM64Ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -506,13 +506,6 @@ func init() {
// LoweredGetCallerSP returns the SP of the caller of the current function.
{name: "LoweredGetCallerSP", reg: gp01, rematerializeable: true},

// MOVDconvert converts between pointers and integers.
// We have a special op for this so as to not confuse GC
// (particularly stack maps). It takes a memory arg so it
// gets correctly ordered with respect to GC safepoints.
// arg0=ptr/int arg1=mem, output=int/ptr
{name: "MOVDconvert", argLength: 2, reg: gp11, asm: "MOVD"},

// Constant flag values. For any comparison, there are 5 possible
// outcomes: the three from the signed total order (<,==,>) and the
// three from the unsigned total order. The == cases overlap.
Expand Down
7 changes: 0 additions & 7 deletions src/cmd/compile/internal/ssa/gen/ARMOps.go
Original file line number Diff line number Diff line change
Expand Up @@ -519,13 +519,6 @@ func init() {
// LoweredGetCallerSP returns the SP of the caller of the current function.
{name: "LoweredGetCallerSP", reg: gp01, rematerializeable: true},

// MOVWconvert converts between pointers and integers.
// We have a special op for this so as to not confuse GC
// (particularly stack maps). It takes a memory arg so it
// gets correctly ordered with respect to GC safepoints.
// arg0=ptr/int arg1=mem, output=int/ptr
{name: "MOVWconvert", argLength: 2, reg: gp11, asm: "MOVW"},

// Constant flag values. For any comparison, there are 5 possible
// outcomes: the three from the signed total order (<,==,>) and the
// three from the unsigned total order. The == cases overlap.
Expand Down
1 change: 0 additions & 1 deletion src/cmd/compile/internal/ssa/gen/MIPS.rules
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,6 @@
// pseudo-ops
(GetClosurePtr) -> (LoweredGetClosurePtr)
(GetCallerSP) -> (LoweredGetCallerSP)
(Convert x mem) -> (MOVWconvert x mem)

(If cond yes no) -> (NE cond yes no)

Expand Down
1 change: 0 additions & 1 deletion src/cmd/compile/internal/ssa/gen/MIPS64.rules
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,6 @@
// pseudo-ops
(GetClosurePtr) -> (LoweredGetClosurePtr)
(GetCallerSP) -> (LoweredGetCallerSP)
(Convert x mem) -> (MOVVconvert x mem)

(If cond yes no) -> (NE cond yes no)

Expand Down
7 changes: 0 additions & 7 deletions src/cmd/compile/internal/ssa/gen/MIPS64Ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -414,13 +414,6 @@ func init() {
// but clobbers R31 (LR) because it's a call
// and R23 (REGTMP).
{name: "LoweredWB", argLength: 3, reg: regInfo{inputs: []regMask{buildReg("R20"), buildReg("R21")}, clobbers: (callerSave &^ gpg) | buildReg("R31")}, clobberFlags: true, aux: "Sym", symEffect: "None"},

// MOVDconvert converts between pointers and integers.
// We have a special op for this so as to not confuse GC
// (particularly stack maps). It takes a memory arg so it
// gets correctly ordered with respect to GC safepoints.
// arg0=ptr/int arg1=mem, output=int/ptr
{name: "MOVVconvert", argLength: 2, reg: gp11, asm: "MOVV"},
}

blocks := []blockData{
Expand Down
7 changes: 0 additions & 7 deletions src/cmd/compile/internal/ssa/gen/MIPSOps.go
Original file line number Diff line number Diff line change
Expand Up @@ -384,13 +384,6 @@ func init() {
// but clobbers R31 (LR) because it's a call
// and R23 (REGTMP).
{name: "LoweredWB", argLength: 3, reg: regInfo{inputs: []regMask{buildReg("R20"), buildReg("R21")}, clobbers: (callerSave &^ gpg) | buildReg("R31")}, clobberFlags: true, aux: "Sym", symEffect: "None"},

// MOVWconvert converts between pointers and integers.
// We have a special op for this so as to not confuse GC
// (particularly stack maps). It takes a memory arg so it
// gets correctly ordered with respect to GC safepoints.
// arg0=ptr/int arg1=mem, output=int/ptr
{name: "MOVWconvert", argLength: 2, reg: gp11, asm: "MOVW"},
}

blocks := []blockData{
Expand Down
1 change: 0 additions & 1 deletion src/cmd/compile/internal/ssa/gen/PPC64.rules
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,6 @@
(InterCall [argwid] entry mem) -> (CALLinter [argwid] entry mem)

// Miscellaneous
(Convert <t> x mem) -> (MOVDconvert <t> x mem)
(GetClosurePtr) -> (LoweredGetClosurePtr)
(GetCallerSP) -> (LoweredGetCallerSP)
(IsNonNil ptr) -> (NotEqual (CMPconst [0] ptr))
Expand Down
3 changes: 0 additions & 3 deletions src/cmd/compile/internal/ssa/gen/PPC64Ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -327,9 +327,6 @@ func init() {
{name: "LoweredRound32F", argLength: 1, reg: fp11, resultInArg0: true, zeroWidth: true},
{name: "LoweredRound64F", argLength: 1, reg: fp11, resultInArg0: true, zeroWidth: true},

// Convert pointer to integer, takes a memory operand for ordering.
{name: "MOVDconvert", argLength: 2, reg: gp11, asm: "MOVD"},

{name: "CALLstatic", argLength: 1, reg: regInfo{clobbers: callerSave}, aux: "SymOff", clobberFlags: true, call: true, symEffect: "None"}, // call static function aux.(*obj.LSym). arg0=mem, auxint=argsize, returns mem
{name: "CALLclosure", argLength: 3, reg: regInfo{inputs: []regMask{callptr, ctxt, 0}, clobbers: callerSave}, aux: "Int64", clobberFlags: true, call: true}, // call function via closure. arg0=codeptr, arg1=closure, arg2=mem, auxint=argsize, returns mem
{name: "CALLinter", argLength: 2, reg: regInfo{inputs: []regMask{callptr}, clobbers: callerSave}, aux: "Int64", clobberFlags: true, call: true}, // call fn by pointer. arg0=codeptr, arg1=mem, auxint=argsize, returns mem
Expand Down
1 change: 0 additions & 1 deletion src/cmd/compile/internal/ssa/gen/S390X.rules
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,6 @@
(InterCall [argwid] entry mem) -> (CALLinter [argwid] entry mem)

// Miscellaneous
(Convert <t> x mem) -> (MOVDconvert <t> x mem)
(IsNonNil p) -> (MOVDNE (MOVDconst [0]) (MOVDconst [1]) (CMPconst p [0]))
(IsInBounds idx len) -> (MOVDLT (MOVDconst [0]) (MOVDconst [1]) (CMPU idx len))
(IsSliceInBounds idx len) -> (MOVDLE (MOVDconst [0]) (MOVDconst [1]) (CMPU idx len))
Expand Down
7 changes: 0 additions & 7 deletions src/cmd/compile/internal/ssa/gen/S390XOps.go
Original file line number Diff line number Diff line change
Expand Up @@ -459,13 +459,6 @@ func init() {
// but clobbers R14 (LR) because it's a call.
{name: "LoweredWB", argLength: 3, reg: regInfo{inputs: []regMask{buildReg("R2"), buildReg("R3")}, clobbers: (callerSave &^ gpg) | buildReg("R14")}, clobberFlags: true, aux: "Sym", symEffect: "None"},

// MOVDconvert converts between pointers and integers.
// We have a special op for this so as to not confuse GC
// (particularly stack maps). It takes a memory arg so it
// gets correctly ordered with respect to GC safepoints.
// arg0=ptr/int arg1=mem, output=int/ptr
{name: "MOVDconvert", argLength: 2, reg: gp11sp, asm: "MOVD"},

// Constant flag values. For any comparison, there are 5 possible
// outcomes: the three from the signed total order (<,==,>) and the
// three from the unsigned total order. The == cases overlap.
Expand Down
5 changes: 4 additions & 1 deletion src/cmd/compile/internal/ssa/gen/genericOps.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,8 +289,11 @@ var genericOps = []opData{
// We have a special op for this so as to not confuse GC
// (particularly stack maps). It takes a memory arg so it
// gets correctly ordered with respect to GC safepoints.
// It gets compiled to nothing, so its result must in the same
// register as its argument. regalloc knows it can use any
// allocatable integer register for OpConvert.
// arg0=ptr/int arg1=mem, output=int/ptr
{name: "Convert", argLength: 2},
{name: "Convert", argLength: 2, zeroWidth: true, resultInArg0: true},

// constants. Constant values are stored in the aux or
// auxint fields.
Expand Down
6 changes: 4 additions & 2 deletions src/cmd/compile/internal/ssa/gen/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,12 @@ func genOp() {
}
if v.resultInArg0 {
fmt.Fprintln(w, "resultInArg0: true,")
if v.reg.inputs[0] != v.reg.outputs[0] {
// OpConvert's register mask is selected dynamically,
// so don't try to check it in the static table.
if v.name != "Convert" && v.reg.inputs[0] != v.reg.outputs[0] {
log.Fatalf("%s: input[0] and output[0] must use the same registers for %s", a.name, v.name)
}
if v.commutative && v.reg.inputs[1] != v.reg.outputs[0] {
if v.name != "Convert" && v.commutative && v.reg.inputs[1] != v.reg.outputs[0] {
log.Fatalf("%s: input[1] and output[0] must use the same registers for %s", a.name, v.name)
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/ssa/lower.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func checkLower(f *Func) {
continue // lowered
}
switch v.Op {
case OpSP, OpSB, OpInitMem, OpArg, OpPhi, OpVarDef, OpVarKill, OpVarLive, OpKeepAlive, OpSelect0, OpSelect1:
case OpSP, OpSB, OpInitMem, OpArg, OpPhi, OpVarDef, OpVarKill, OpVarLive, OpKeepAlive, OpSelect0, OpSelect1, OpConvert:
continue // ok not to lower
case OpGetG:
if f.Config.hasGReg {
Expand Down
Loading

0 comments on commit 8871c93

Please sign in to comment.