Repository: tetratelabs/wazero
Pull request #1394 (2023-04-24 00:46:55)

asm/amd64: removes nodeImpl.forwardJumpTarget
```
goos: linux
goarch: amd64
pkg: github.com/tetratelabs/wazero/internal/integration_test/bench
cpu: AMD Ryzen 9 3950X 16-Core Processor
                                    │   old.txt   │              new.txt              │
                                    │   sec/op    │   sec/op     vs base              │
Compilation/without_extern_cache-32   3.820m ± 3%   3.777m ± 2%       ~ (p=0.535 n=7)
Compilation_sqlite3/compiler-32       395.2m ± 2%   388.6m ± 2%  -1.66% (p=0.038 n=7)
geomean                               38.85m        38.31m       -1.40%

                                    │   old.txt    │              new.txt               │
                                    │     B/op     │     B/op      vs base              │
Compilation/without_extern_cache-32   664.4Ki ± 0%   664.5Ki ± 0%  +0.02% (p=0.038 n=7)
Compilation_sqlite3/compiler-32       49.40Mi ± 0%   49.40Mi ± 0%       ~ (p=0.476 n=7)
geomean                               5.661Mi        5.662Mi       +0.01%

                                    │   old.txt   │               new.txt               │
                                    │  allocs/op  │  allocs/op   vs base                │
Compilation/without_extern_cache-32    852.0 ± 0%    852.0 ± 0%       ~ (p=1.000 n=7) ¹
Compilation_sqlite3/compiler-32       28.70k ± 0%   28.70k ± 0%       ~ (p=0.681 n=7)
geomean                               4.945k        4.945k       +0.00%
```


Patch:
diff --git a/internal/asm/amd64/impl.go b/internal/asm/amd64/impl.go
index 6b8f7bc3..f35c107c 100644
--- a/internal/asm/amd64/impl.go
+++ b/internal/asm/amd64/impl.go
@@ -14,7 +14,7 @@ import (
 type nodeImpl struct {
 	instruction asm.Instruction
 
-	offsetInBinaryField asm.NodeOffsetInBinary // Field suffix to dodge conflict with OffsetInBinary
+	offsetInBinary asm.NodeOffsetInBinary
 
 	// jumpTarget holds the target node in the linked for the jump-kind instruction.
 	jumpTarget *nodeImpl
@@ -36,9 +36,6 @@ type nodeImpl struct {
 
 	// forwardJumpOrigins hold all the nodes trying to jump into this node as a singly linked list. In other words, all the nodes with .jumpTarget == this.
 	forwardJumpOrigins *nodeImpl
-	// jumpOriginsHead true if this is the target of all the nodes in the singly linked list jumpOrigins,
-	// and can be traversed from this nodeImpl's forwardJumpOrigins.
-	forwardJumpTarget bool
 	// staticConstReferrersAdded true if this node is already added into AssemblerImpl.staticConstReferrers.
 	// Only used when staticConst is not nil. Through re-assembly, we might end up adding multiple times which causes unnecessary
 	// allocations, so we use this flag to do it once.
@@ -96,7 +93,7 @@ func (n *nodeImpl) AssignSourceConstant(value asm.ConstantValue) {
 
 // OffsetInBinary implements asm.Node.OffsetInBinary.
 func (n *nodeImpl) OffsetInBinary() asm.NodeOffsetInBinary {
-	return n.offsetInBinaryField
+	return n.offsetInBinary
 }
 
 // String implements fmt.Stringer.
@@ -394,7 +391,7 @@ func (a *AssemblerImpl) Assemble() ([]byte, error) {
 	// Continue encoding until we are not forced to re-assemble which happens when
 	// a short relative jump ends up the offset larger than 8-bit length.
 	for {
-		err := a.Encode()
+		err := a.encode()
 		if err != nil {
 			return nil, err
 		}
@@ -462,13 +459,8 @@ func (a *AssemblerImpl) initializeNodesForEncoding() {
 					prev.next = nop
 					nop.next = target
 					target.prev = nop
-					// Assign this as a jump destination.
-					nop.forwardJumpTarget = true
 					n.jumpTarget = nop
 					target = nop
-				} else {
-					// Otherwise, simply we mark this target as the forward jump target.
-					target.forwardJumpTarget = true
 				}
 
 				// We add this node `n` into the end of the linked list (.forwardJumpOrigins) beginning from the `target.forwardJumpOrigins`.
@@ -480,7 +472,7 @@ func (a *AssemblerImpl) initializeNodesForEncoding() {
 	}
 }
 
-func (a *AssemblerImpl) Encode() (err error) {
+func (a *AssemblerImpl) encode() (err error) {
 	for n := a.root; n != nil; n = n.next {
 		// If an instruction needs NOP padding, we do so before encoding it.
 		// https://www.intel.com/content/dam/support/us/en/documents/processors/mitigations-jump-conditional-code-erratum.pdf
@@ -491,16 +483,17 @@ func (a *AssemblerImpl) Encode() (err error) {
 		}
 
 		// After the padding, we can finalize the offset of this instruction in the binary.
-		n.offsetInBinaryField = uint64(a.buf.Len())
+		n.offsetInBinary = uint64(a.buf.Len())
 
 		if err = a.encodeNode(n); err != nil {
 			return
 		}
 
-		err = a.ResolveForwardRelativeJumps(n)
-		if err != nil {
-			err = fmt.Errorf("invalid relative forward jumps: %w", err)
-			break
+		if n.forwardJumpOrigins != nil {
+			if err = a.resolveForwardRelativeJumps(n); err != nil {
+				err = fmt.Errorf("invalid relative forward jumps: %w", err)
+				break
+			}
 		}
 
 		a.maybeFlushConstants(n.next == nil)
@@ -554,7 +547,7 @@ func (a *AssemblerImpl) maybeNOPPadding(n *nodeImpl) (err error) {
 		prevLen := a.buf.Len()
 
 		// Assign the temporary offset which may or may not be correct depending on the padding decision.
-		n.offsetInBinaryField = uint64(prevLen)
+		n.offsetInBinary = uint64(prevLen)
 
 		// Encode the node and get the instruction length.
 		if err = a.encodeNode(n); err != nil {
@@ -1099,10 +1092,7 @@ var relativeJumpOpcodes = [...]relativeJumpOpcode{
 	JMP: {short: []byte{0xeb}, long: []byte{0xe9}},
 }
 
-func (a *AssemblerImpl) ResolveForwardRelativeJumps(target *nodeImpl) (err error) {
-	if !target.forwardJumpTarget {
-		return
-	}
+func (a *AssemblerImpl) resolveForwardRelativeJumps(target *nodeImpl) (err error) {
 	offsetInBinary := int64(target.OffsetInBinary())
 	origin := target.forwardJumpOrigins
 	for ; origin != nil; origin = origin.forwardJumpOrigins {


Test patch:
diff --git a/internal/asm/amd64/impl_1_test.go b/internal/asm/amd64/impl_1_test.go
index 3f065d14..861e6dbb 100644
--- a/internal/asm/amd64/impl_1_test.go
+++ b/internal/asm/amd64/impl_1_test.go
@@ -28,16 +28,15 @@ func TestNodePool_allocNode(t *testing.T) {
 
 	// Taint the existing content on the page.
 	np.pages[np.page][np.pos] = nodeImpl{
-		offsetInBinaryField: 10,
-		jumpTarget:          &nodeImpl{},
-		flag:                nodeFlagInitializedForEncoding,
-		next:                &nodeImpl{},
-		staticConst:         asm.NewStaticConst([]byte{1, 2}),
+		offsetInBinary: 10,
+		jumpTarget:     &nodeImpl{},
+		flag:           nodeFlagInitializedForEncoding,
+		next:           &nodeImpl{},
+		staticConst:    asm.NewStaticConst([]byte{1, 2}),
 		readInstructionAddressBeforeTargetInstruction: RET,
-		forwardJumpTarget: true,
-		arg:               1,
-		types:             operandTypesConstToRegister,
-		srcReg:            RegBX, dstReg: RegBX,
+		arg:    1,
+		types:  operandTypesConstToRegister,
+		srcReg: RegBX, dstReg: RegBX,
 		srcConst: 1234, dstConst: 1234,
 		srcMemIndex: RegBX, dstMemIndex: RegBX,
 		srcMemScale: 0xf, dstMemScale: 0xf,
@@ -148,7 +147,7 @@ func TestAssemblerImpl_Assemble(t *testing.T) {
 		a.initializeNodesForEncoding()
 
 		// For the first encoding, we must be forced to reassemble.
-		err := a.Encode()
+		err := a.encode()
 		require.NoError(t, err)
 		require.True(t, a.forceReAssemble)
 	})
diff --git a/internal/asm/amd64/impl_4_test.go b/internal/asm/amd64/impl_4_test.go
index a5542436..a583bdab 100644
--- a/internal/asm/amd64/impl_4_test.go
+++ b/internal/asm/amd64/impl_4_test.go
@@ -327,14 +327,14 @@ func TestAssemblerImpl_encodeReadInstructionAddress(t *testing.T) {
 		a.CompileStandAlone(CDQ)
 
 		for n := a.root; n != nil; n = n.next {
-			n.offsetInBinaryField = uint64(a.buf.Len())
+			n.offsetInBinary = uint64(a.buf.Len())
 
 			err := a.encodeNode(n)
 			require.NoError(t, err)
 		}
 
 		targetNode := a.current
-		targetNode.offsetInBinaryField = uint64(math.MaxInt64)
+		targetNode.offsetInBinary = uint64(math.MaxInt64)
 
 		n := a.readInstructionAddressNodes[0]
 		err := a.finalizeReadInstructionAddressNode(nil, n)
diff --git a/internal/asm/amd64/impl_5_test.go b/internal/asm/amd64/impl_5_test.go
index 41cdfa96..5409c9ec 100644
--- a/internal/asm/amd64/impl_5_test.go
+++ b/internal/asm/amd64/impl_5_test.go
@@ -934,10 +934,10 @@ func TestAssemblerImpl_ResolveForwardRelativeJumps(t *testing.T) {
 	t.Run("long jump", func(t *testing.T) {
 		t.Run("error", func(t *testing.T) {
 			originOffset, targetOffset := uint64(0), uint64(math.MaxInt64)
-			origin := &nodeImpl{instruction: JMP, offsetInBinaryField: originOffset}
-			target := &nodeImpl{offsetInBinaryField: targetOffset, forwardJumpOrigins: origin, forwardJumpTarget: true}
+			origin := &nodeImpl{instruction: JMP, offsetInBinary: originOffset}
+			target := &nodeImpl{offsetInBinary: targetOffset, forwardJumpOrigins: origin}
 			a := NewAssembler()
-			err := a.ResolveForwardRelativeJumps(target)
+			err := a.resolveForwardRelativeJumps(target)
 			require.EqualError(t, err, "too large jump offset 9223372036854775802 for encoding JMP")
 		})
 		t.Run("ok", func(t *testing.T) {
@@ -962,14 +962,14 @@ func TestAssemblerImpl_ResolveForwardRelativeJumps(t *testing.T) {
 
 			for _, tt := range tests {
 				tc := tt
-				origin := &nodeImpl{instruction: tc.instruction, offsetInBinaryField: originOffset}
-				target := &nodeImpl{offsetInBinaryField: tc.targetOffset, forwardJumpOrigins: origin, forwardJumpTarget: true}
+				origin := &nodeImpl{instruction: tc.instruction, offsetInBinary: originOffset}
+				target := &nodeImpl{offsetInBinary: tc.targetOffset, forwardJumpOrigins: origin}
 				a := NewAssembler()
 
 				// Grow the capacity of buffer so that we could put the offset.
 				a.buf.Write([]byte{0, 0, 0, 0, 0, 0}) // Relative long jumps are at most 6 bytes.
 
-				err := a.ResolveForwardRelativeJumps(target)
+				err := a.resolveForwardRelativeJumps(target)
 				require.NoError(t, err)
 
 				actual := binary.LittleEndian.Uint32(a.buf.Bytes()[tc.writtenOffsetIndexInBinary:])
@@ -1006,12 +1006,12 @@ func TestAssemblerImpl_ResolveForwardRelativeJumps(t *testing.T) {
 
 			for _, tt := range tests {
 				tc := tt
-				origin := &nodeImpl{instruction: tc.instruction, offsetInBinaryField: originOffset, flag: nodeFlagShortForwardJump}
-				target := &nodeImpl{offsetInBinaryField: tc.targetOffset, forwardJumpOrigins: origin, forwardJumpTarget: true}
+				origin := &nodeImpl{instruction: tc.instruction, offsetInBinary: originOffset, flag: nodeFlagShortForwardJump}
+				target := &nodeImpl{offsetInBinary: tc.targetOffset, forwardJumpOrigins: origin}
 				origin.jumpTarget = target
 
 				a := NewAssembler()
-				err := a.ResolveForwardRelativeJumps(target)
+				err := a.resolveForwardRelativeJumps(target)
 				require.NoError(t, err)
 
 				require.True(t, a.forceReAssemble)
@@ -1037,8 +1037,8 @@ func TestAssemblerImpl_ResolveForwardRelativeJumps(t *testing.T) {
 
 			for _, tt := range tests {
 				tc := tt
-				origin := &nodeImpl{instruction: tc.instruction, offsetInBinaryField: originOffset, flag: nodeFlagShortForwardJump}
-				target := &nodeImpl{offsetInBinaryField: tc.targetOffset, forwardJumpOrigins: origin, forwardJumpTarget: true}
+				origin := &nodeImpl{instruction: tc.instruction, offsetInBinary: originOffset, flag: nodeFlagShortForwardJump}
+				target := &nodeImpl{offsetInBinary: tc.targetOffset, forwardJumpOrigins: origin}
 				origin.jumpTarget = target
 
 				a := NewAssembler()
@@ -1046,7 +1046,7 @@ func TestAssemblerImpl_ResolveForwardRelativeJumps(t *testing.T) {
 				// Grow the capacity of buffer so that we could put the offset.
 				a.buf.Write([]byte{0, 0}) // Relative short jumps are of 2 bytes.
 
-				err := a.ResolveForwardRelativeJumps(target)
+				err := a.resolveForwardRelativeJumps(target)
 				require.NoError(t, err)
 
 				actual := a.buf.Bytes()[1] // For short jumps, the opcode has one opcode so the offset is writte at 2nd byte.
diff --git a/internal/asm/amd64/impl_7_test.go b/internal/asm/amd64/impl_7_test.go
index 39384a57..81a2c2bb 100644
--- a/internal/asm/amd64/impl_7_test.go
+++ b/internal/asm/amd64/impl_7_test.go
@@ -485,10 +485,10 @@ func TestAssemblerImpl_encodeNoneToBranch_backward_jumps(t *testing.T) {
 		targetOffsetInBinaryField := uint64(0)
 		OffsetInBinaryField := uint64(math.MaxInt32)
 		node := &nodeImpl{
-			instruction:         JMP,
-			jumpTarget:          &nodeImpl{offsetInBinaryField: targetOffsetInBinaryField},
-			flag:                nodeFlagBackwardJump,
-			offsetInBinaryField: OffsetInBinaryField,
+			instruction:    JMP,
+			jumpTarget:     &nodeImpl{offsetInBinary: targetOffsetInBinaryField},
+			flag:           nodeFlagBackwardJump,
+			offsetInBinary: OffsetInBinaryField,
 		}
 		err := a.encodeRelativeJump(node)
 		require.Error(t, err)