Repository: mmcloughlin/avo (go)

Issue:
pass: allow labels following each other
When generating complex stuff it is a bit annoying that it isn't possible to have two labels follow each other.

[Example](https://github.com/klauspost/compress/blob/7ca389b3e8e5bb3e3fb12deb7d28c9b24f01443b/s2/gen.go#L1060) where I need to be able to jump back to the beginning of a piece of inlined code.

This gets annoying when I have [a caller](https://github.com/klauspost/compress/blob/7ca389b3e8e5bb3e3fb12deb7d28c9b24f01443b/s2/gen.go#L675) that may or may not have code between itself and the previous label. This forces me to insert a [`NOP`](https://github.com/klauspost/compress/blob/7ca389b3e8e5bb3e3fb12deb7d28c9b24f01443b/s2/gen.go#L1062).

I am wondering if there are technical problems in lifting this restriction?

Pull request description:
pass: allow consecutive labels
Fixes #122

Patch:
diff --git a/pass/cfg.go b/pass/cfg.go
index d8e8fd4..f308e2e 100644
--- a/pass/cfg.go
+++ b/pass/cfg.go
@@ -11,26 +11,22 @@ import (
 // label name to the following instruction.
 func LabelTarget(fn *ir.Function) error {
 	target := map[ir.Label]*ir.Instruction{}
-	var empty ir.Label
-	pending := empty
+	var pending []ir.Label
 	for _, node := range fn.Nodes {
 		switch n := node.(type) {
 		case ir.Label:
-			if pending != empty {
-				return fmt.Errorf("expected instruction following label %q", pending)
-			}
-			pending = n
-			if _, found := target[pending]; found {
-				return fmt.Errorf("duplicate label \"%s\"", pending)
+			if _, found := target[n]; found {
+				return fmt.Errorf("duplicate label \"%s\"", n)
 			}
+			pending = append(pending, n)
 		case *ir.Instruction:
-			if pending != empty {
-				target[pending] = n
-				pending = empty
+			for _, label := range pending {
+				target[label] = n
 			}
+			pending = nil
 		}
 	}
-	if pending != empty {
+	if len(pending) != 0 {
 		return errors.New("function ends with label")
 	}
 	fn.LabelTarget = target


Test patch:
diff --git a/pass/cfg_test.go b/pass/cfg_test.go
index 5dc8857..e7a2bb1 100644
--- a/pass/cfg_test.go
+++ b/pass/cfg_test.go
@@ -58,16 +58,25 @@ func TestLabelTargetEndsWithLabel(t *testing.T) {
 	}
 }
 
-func TestLabelTargetInstructionFollowLabel(t *testing.T) {
-	f := ir.NewFunction("expectinstafterlabel")
+func TestLabelTargetConsecutiveLabels(t *testing.T) {
+	i := &ir.Instruction{Opcode: "A"}
+
+	f := ir.NewFunction("consecutivelabels")
 	f.AddLabel(ir.Label("lblA"))
 	f.AddLabel(ir.Label("lblB"))
-	f.AddInstruction(&ir.Instruction{Opcode: "A"})
+	f.AddInstruction(i)
 
-	err := LabelTarget(f)
+	expect := map[ir.Label]*ir.Instruction{
+		"lblA": i,
+		"lblB": i,
+	}
 
-	if err == nil || err.Error() != "expected instruction following label \"lblA\"" {
-		t.Fatalf("expected error when label is not followed by instruction; got %v", err)
+	if err := LabelTarget(f); err != nil {
+		t.Fatal(err)
+	}
+
+	if !reflect.DeepEqual(expect, f.LabelTarget) {
+		t.Fatalf("incorrect LabelTarget value\ngot=%#v\nexpext=%#v\n", f.LabelTarget, expect)
 	}
 }
 
diff --git a/tests/fixedbugs/issue122/asm.go b/tests/fixedbugs/issue122/asm.go
new file mode 100644
index 0000000..95a5c47
--- /dev/null
+++ b/tests/fixedbugs/issue122/asm.go
@@ -0,0 +1,42 @@
+// +build ignore
+
+package main
+
+import (
+	. "github.com/mmcloughlin/avo/build"
+	. "github.com/mmcloughlin/avo/operand"
+)
+
+func main() {
+	TEXT("Triangle", NOSPLIT, "func(n uint64) uint64")
+	Doc("Triangle computes the nth triangle number.")
+	n := Load(Param("n"), GP64())
+
+	Comment("Initialize sum register to zero.")
+	s := GP64()
+	XORQ(s, s)
+
+	// Use two labels for the top of the loop.
+	Label("loop_even")
+	Label("loop_odd")
+	Comment("Loop until n is zero.")
+	CMPQ(n, Imm(0))
+	JE(LabelRef("done"))
+
+	Comment("Add n to sum.")
+	ADDQ(n, s)
+
+	Comment("Decrement n.")
+	DECQ(n)
+
+	Comment("Jump to one of the loop labels depending on parity.")
+	TESTQ(U32(1), n)
+	JZ(LabelRef("loop_even"))
+	JMP(LabelRef("loop_odd"))
+
+	Label("done")
+	Comment("Store sum to return value.")
+	Store(s, ReturnIndex(0))
+	RET()
+	Generate()
+}
diff --git a/tests/fixedbugs/issue122/doc.go b/tests/fixedbugs/issue122/doc.go
new file mode 100644
index 0000000..e5d6893
--- /dev/null
+++ b/tests/fixedbugs/issue122/doc.go
@@ -0,0 +1,2 @@
+// Package issue122 tests consecutive labels.
+package issue122
diff --git a/tests/fixedbugs/issue122/issue122.s b/tests/fixedbugs/issue122/issue122.s
new file mode 100644
index 0000000..c053da2
--- /dev/null
+++ b/tests/fixedbugs/issue122/issue122.s
@@ -0,0 +1,32 @@
+// Code generated by command: go run asm.go -out issue122.s -stubs stub.go. DO NOT EDIT.
+
+#include "textflag.h"
+
+// func Triangle(n uint64) uint64
+TEXT ·Triangle(SB), NOSPLIT, $0-16
+	MOVQ n+0(FP), AX
+
+	// Initialize sum register to zero.
+	XORQ CX, CX
+
+loop_even:
+loop_odd:
+	// Loop until n is zero.
+	CMPQ AX, $0x00
+	JE   done
+
+	// Add n to sum.
+	ADDQ AX, CX
+
+	// Decrement n.
+	DECQ AX
+
+	// Jump to one of the loop labels depending on parity.
+	TESTQ $0x00000001, AX
+	JZ    loop_even
+	JMP   loop_odd
+
+done:
+	// Store sum to return value.
+	MOVQ CX, ret+8(FP)
+	RET
diff --git a/tests/fixedbugs/issue122/issue122_test.go b/tests/fixedbugs/issue122/issue122_test.go
new file mode 100644
index 0000000..7a93a8b
--- /dev/null
+++ b/tests/fixedbugs/issue122/issue122_test.go
@@ -0,0 +1,16 @@
+package issue122
+
+import (
+	"testing"
+)
+
+//go:generate go run asm.go -out issue122.s -stubs stub.go
+
+func TestTriangle(t *testing.T) {
+	expect := func(n uint64) uint64 { return n * (n + 1) / 2 }
+	for n := uint64(1); n < 42; n++ {
+		if got := Triangle(n); expect(n) != got {
+			t.Fatalf("Triangle(%v) = %v; expect %v", n, got, expect(n))
+		}
+	}
+}
diff --git a/tests/fixedbugs/issue122/stub.go b/tests/fixedbugs/issue122/stub.go
new file mode 100644
index 0000000..e936aca
--- /dev/null
+++ b/tests/fixedbugs/issue122/stub.go
@@ -0,0 +1,6 @@
+// Code generated by command: go run asm.go -out issue122.s -stubs stub.go. DO NOT EDIT.
+
+package issue122
+
+// Triangle computes the nth triangle number.
+func Triangle(n uint64) uint64
diff --git a/tests/labels/asm.go b/tests/labels/asm.go
index 443e2df..266159e 100644
--- a/tests/labels/asm.go
+++ b/tests/labels/asm.go
@@ -12,7 +12,8 @@ func main() {
 	TEXT("Labels", NOSPLIT, "func() uint64")
 	XORQ(RAX, RAX)
 	INCQ(RAX)
-	Label("neverused")
+	Label("never_used")
+	Label("consecutive_label_also_never_used")
 	INCQ(RAX)
 	INCQ(RAX)
 	INCQ(RAX)