From 9d65578b83f0d52f0d2f526212dd3b4ee1a5f031 Mon Sep 17 00:00:00 2001 From: fanzha02 Date: Wed, 30 Jun 2021 10:51:54 +0800 Subject: cmd/compile: fix typos in document Correct "a2Spill" to "a3Spill" Change-Id: I6ac4c45973dfaeb16d3a90d835589b6af1aefe1d Reviewed-on: https://go-review.googlesource.com/c/go/+/331850 Trust: fannie zhang Reviewed-by: Cherry Mui --- src/cmd/compile/abi-internal.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/cmd/compile') diff --git a/src/cmd/compile/abi-internal.md b/src/cmd/compile/abi-internal.md index 1ae3c2538f..2bb4055083 100644 --- a/src/cmd/compile/abi-internal.md +++ b/src/cmd/compile/abi-internal.md @@ -233,7 +233,7 @@ stack frame is laid out in the following sequence: r1.x uintptr r1.y [2]uintptr a1Spill uint8 - a2Spill uint8 + a3Spill uint8 _ [6]uint8 // alignment padding In the stack frame, only the `a2` field is initialized on entry; the -- cgit v1.3-5-g9baa From eb437ba92cbb08a86ae064cbd7376c4a8e80485b Mon Sep 17 00:00:00 2001 From: go101 Date: Thu, 1 Jul 2021 14:25:45 +0000 Subject: cmd/compile: make stack value size threshold comparisons consistent Consistency is beautiful. Change-Id: Ib110dcff0ce2fa87b5576c79cd79c83aab385a7c GitHub-Last-Rev: b8758f8ae02cb025267aa87ebc5c2f9b4c32e742 GitHub-Pull-Request: golang/go#47011 Reviewed-on: https://go-review.googlesource.com/c/go/+/332230 Reviewed-by: Keith Randall Reviewed-by: Robert Griesemer Run-TryBot: Keith Randall TryBot-Result: Go Bot --- src/cmd/compile/internal/escape/escape.go | 8 ++++---- src/cmd/compile/internal/walk/builtin.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) (limited to 'src/cmd/compile') diff --git a/src/cmd/compile/internal/escape/escape.go b/src/cmd/compile/internal/escape/escape.go index 3ac7ff1ebe..cd56f07b61 100644 --- a/src/cmd/compile/internal/escape/escape.go +++ b/src/cmd/compile/internal/escape/escape.go @@ -2013,14 +2013,14 @@ func HeapAllocReason(n ir.Node) string { return "too large for stack" } - if (n.Op() == ir.ONEW || n.Op() == ir.OPTRLIT) && n.Type().Elem().Width >= ir.MaxImplicitStackVarSize { + if (n.Op() == ir.ONEW || n.Op() == ir.OPTRLIT) && n.Type().Elem().Width > ir.MaxImplicitStackVarSize { return "too large for stack" } - if n.Op() == ir.OCLOSURE && typecheck.ClosureType(n.(*ir.ClosureExpr)).Size() >= ir.MaxImplicitStackVarSize { + if n.Op() == ir.OCLOSURE && typecheck.ClosureType(n.(*ir.ClosureExpr)).Size() > ir.MaxImplicitStackVarSize { return "too large for stack" } - if n.Op() == ir.OCALLPART && typecheck.PartialCallType(n.(*ir.SelectorExpr)).Size() >= ir.MaxImplicitStackVarSize { + if n.Op() == ir.OCALLPART && typecheck.PartialCallType(n.(*ir.SelectorExpr)).Size() > ir.MaxImplicitStackVarSize { return "too large for stack" } @@ -2033,7 +2033,7 @@ func HeapAllocReason(n ir.Node) string { if !ir.IsSmallIntConst(r) { return "non-constant size" } - if t := n.Type(); t.Elem().Width != 0 && ir.Int64Val(r) >= ir.MaxImplicitStackVarSize/t.Elem().Width { + if t := n.Type(); t.Elem().Width != 0 && ir.Int64Val(r) > ir.MaxImplicitStackVarSize/t.Elem().Width { return "too large for stack" } } diff --git a/src/cmd/compile/internal/walk/builtin.go b/src/cmd/compile/internal/walk/builtin.go index 1f08e4d312..14efc05e32 100644 --- a/src/cmd/compile/internal/walk/builtin.go +++ b/src/cmd/compile/internal/walk/builtin.go @@ -489,7 +489,7 @@ func walkNew(n *ir.UnaryExpr, init *ir.Nodes) ir.Node { base.Errorf("%v can't be allocated in Go; it is incomplete (or unallocatable)", n.Type().Elem()) } if n.Esc() == ir.EscNone { - if t.Size() >= ir.MaxImplicitStackVarSize { + if t.Size() > ir.MaxImplicitStackVarSize { base.Fatalf("large ONEW with EscNone: %v", n) } return stackTempAddr(init, t) -- cgit v1.3-5-g9baa From ef8ae82b37657ab788f490bd757ad1b5592b952f Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Thu, 1 Jul 2021 11:22:02 -0400 Subject: cmd/compile: fix bug in dwarf-gen var location generation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch fixes a bug in the SSA back end's DWARF generation code that determines variable locations / lifetimes. The code in question was written to handle sequences of initial pseudo-ops (zero width instructions such as OpPhi, OpArg, etc) in a basic block, detecting these ops at the start of a block and then treating the values specially when emitting ranges for the variables in those values. The logic in this code wasn't quite correct, meaning that a flag variable wasn't being set properly to record the presence of a block of zero-width value-bearing ops, leading to incorrect or missing DWARF locations for register params. Also in this patch is a tweak to some sanity-checking code intended to catch scheduling problems with OpArg/OpPhi etc. The checks need to allow for the possibility of an Arg op scheduled after a spill of an incoming register param inserted by the register allocator. Example: b1: v13 = ArgIntReg {p1+16} [2] : CX v14 = ArgIntReg {p2+16} [5] : R8 v38 = ArgIntReg {p3+16} [8] : R11 v35 = ArgIntReg {p1+0} [0] : AX v15 = StoreReg v35 : .autotmp_4[int] v40 = Arg {p4} [16] : p4+16[int] v1 = InitMem v3 = SB : SB v18 = CMPQ v14 v13 NE v18 → b3 b2 (unlikely) (18) Here the register allocator has decided to spill v35, meaning that the OpArg v40 is no longer going to be positioned prior to all other non-zero-width ops; this is a valid scenario and needs to be handled properly by the debug code. Fixes #46425. Change-Id: I239b3ad56a9c1b8ebf68af42e1f57308293ed7e6 Reviewed-on: https://go-review.googlesource.com/c/go/+/332269 Trust: Than McIntosh Reviewed-by: Cherry Mui Run-TryBot: Than McIntosh TryBot-Result: Go Bot --- src/cmd/compile/internal/ssa/debug.go | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) (limited to 'src/cmd/compile') diff --git a/src/cmd/compile/internal/ssa/debug.go b/src/cmd/compile/internal/ssa/debug.go index eaa94975ec..8e2872363b 100644 --- a/src/cmd/compile/internal/ssa/debug.go +++ b/src/cmd/compile/internal/ssa/debug.go @@ -1115,8 +1115,14 @@ func (state *debugState) buildLocationLists(blockLocs []*BlockDebug) { continue } + mustBeFirst := func(v *Value) bool { + return v.Op == OpPhi || v.Op.isLoweredGetClosurePtr() || + v.Op == OpArgIntReg || v.Op == OpArgFloatReg + } + zeroWidthPending := false - apcChangedSize := 0 // size of changedVars for leading Args, Phi, ClosurePtr + blockPrologComplete := false // set to true at first non-zero-width op + apcChangedSize := 0 // size of changedVars for leading Args, Phi, ClosurePtr // expect to see values in pattern (apc)* (zerowidth|real)* for _, v := range b.Values { slots := state.valueNames[v.ID] @@ -1125,16 +1131,16 @@ func (state *debugState) buildLocationLists(blockLocs []*BlockDebug) { if opcodeTable[v.Op].zeroWidth { if changed { - if hasAnyArgOp(v) || v.Op == OpPhi || v.Op.isLoweredGetClosurePtr() { + if mustBeFirst(v) || v.Op == OpArg { // These ranges begin at true beginning of block, not after first instruction - if zeroWidthPending { - panic(fmt.Errorf("Unexpected op '%s' mixed with OpArg/OpPhi/OpLoweredGetClosurePtr at beginning of block %s in %s\n%s", v.LongString(), b, b.Func.Name, b.Func)) + if blockPrologComplete && mustBeFirst(v) { + panic(fmt.Errorf("Unexpected placement of op '%s' appearing after non-pseudo-op at beginning of block %s in %s\n%s", v.LongString(), b, b.Func.Name, b.Func)) } apcChangedSize = len(state.changedVars.contents()) + // Other zero-width ops must wait on a "real" op. + zeroWidthPending = true continue } - // Other zero-width ops must wait on a "real" op. - zeroWidthPending = true } continue } @@ -1145,6 +1151,7 @@ func (state *debugState) buildLocationLists(blockLocs []*BlockDebug) { // Not zero-width; i.e., a "real" instruction. zeroWidthPending = false + blockPrologComplete = true for i, varID := range state.changedVars.contents() { if i < apcChangedSize { // buffered true start-of-block changes state.updateVar(VarID(varID), v.Block, BlockStart) -- cgit v1.3-5-g9baa