fix(format): preserve square brackets in SIMD operands

Assisted-by: GLM 5.3 Flash
2026-09-20 09:58:25 +02:00
parent 9629897202
commit c66a47973a
3 changed files with 172 additions and 10 deletions
@@ -300,6 +300,26 @@ func wouldMerge(prev, cur token.Token) bool {
 	return len(kinds) != 2 || kinds[0] != prev.Kind || kinds[1] != cur.Kind
 }

+// isOperandBracket reports whether t is one of the square-bracket tokens the
+// lexer emits, as Illegal tokens carrying their spelling, for the arm64 and
+// loong64 register lists and element selectors that valid GAsm source
+// contains.
+func isOperandBracket(t token.Token) bool {
+	return t.Kind == token.Illegal && (t.Text == "[" || t.Text == "]")
+}
+
+// isOpenBracket reports whether t is the '[' of a register list or element
+// selector.
+func isOpenBracket(t token.Token) bool {
+	return t.Kind == token.Illegal && t.Text == "["
+}
+
+// isCloseBracket reports whether t is the ']' that closes a register list or
+// element selector.
+func isCloseBracket(t token.Token) bool {
+	return t.Kind == token.Illegal && t.Text == "]"
+}
+
 // spaceBetween decides whether a single space separates prev and cur.
 func spaceBetween(prev, cur token.Token) bool {
 	// '/' beside '/' or '*' would form a comment opener in the output and
@@ -308,6 +328,22 @@ func spaceBetween(prev, cur token.Token) bool {
 		return true
 	}
 	switch cur.Kind {
+	case token.Illegal:
+		// A closing bracket always glues to the text it closes.  An opening
+		// bracket glues to the operand it extends (V31.B[15]) but takes its
+		// own space after a comma, a mnemonic or an operator, exactly like
+		// the parenthesis rule below.  Any other Illegal spelling is stray.
+		if isCloseBracket(cur) {
+			return false
+		}
+		if isOpenBracket(cur) {
+			switch prev.Kind {
+			case token.Ident, token.Number, token.RParen, token.RAngle:
+				return false
+			}
+			return true
+		}
+		return true
 	case token.RParen:
 		return false
 	case token.Comma:
@@ -328,6 +364,10 @@ func spaceBetween(prev, cur token.Token) bool {
 		}
 	}
 	switch prev.Kind {
+	case token.Illegal:
+		// '[' opens a bracket group and glues to what follows; ']' closes
+		// one, and what comes next takes its own space.
+		return !isOpenBracket(prev)
 	case token.LParen, token.Star, token.Plus, token.Minus, token.Slash, token.Pipe:
 		return false
 	case token.Dollar:
@@ -358,8 +398,15 @@ func splitLines(toks []token.Token) [][]token.Token {
 			// Illegal tokens carry no canonical spelling: the parser
 			// reports them as errors where they matter, and the formatter
 			// drops them so that a stray character cannot survive into the
-			// output and make the next pass render a different file.
-			continue
+			// output and make the next pass render a different file.  The
+			// square brackets of the arm64 and loong64 vector syntaxes are
+			// the one exception: the lexer gives them no dedicated kind,
+			// but a register list [V0.B16, V1.B16] and an element selector
+			// V0.B[3] are valid, load-bearing source, so their tokens stay
+			// in the stream and renderOps glues them back where they were.
+			if !isOperandBracket(t) {
+				continue
+			}
 		}
 		if t.Kind == token.Newline {
 			lines = append(lines, cur)
@@ -153,6 +153,121 @@ func TestOperandSpacing(t *testing.T) {
 	}
 }

+// TestVectorBracketSpacing pins the square-bracket operand forms of the
+// arm64 and loong64 vector syntaxes.  The lexer emits '[' and ']' as Illegal
+// tokens carrying their spelling, and renderOps must glue them back exactly
+// where they were: a register list and an element selector are load-bearing
+// operands the assembler reads out of the operand text, so no bracket may be
+// dropped, and the canonical spelling inside the brackets is tight.
+func TestVectorBracketSpacing(t *testing.T) {
+	cases := map[string]string{
+		// Register lists of one to four registers.
+		"[V21.B16]":                            "[V21.B16]",
+		"[V17.B16, V18.B16]":                   "[V17.B16, V18.B16]",
+		"[V18.D1, V19.D1, V20.D1]":             "[V18.D1, V19.D1, V20.D1]",
+		"[V14.B16, V15.B16, V16.B16, V17.B16]": "[V14.B16, V15.B16, V16.B16, V17.B16]",
+		// Element selectors.
+		"V31.B[15]":            "V31.B[15]",
+		"V19.S[0]":             "V19.S[0]",
+		"V1.D[1]":              "V1.D[1]",
+		"V11.B[11], V16.B[12]": "V11.B[11], V16.B[12]",
+		// Lists beside address operands, on either side.
+		"32(R1), [V2.B16, V3.B16]": "32(R1), [V2.B16, V3.B16]",
+		"[V2.S4, V3.S4], (R14)":    "[V2.S4, V3.S4], (R14)",
+		"(R24), [V18.D1, V19.D1]":  "(R24), [V18.D1, V19.D1]",
+		// A spaced spelling canonicalises to the tight one.
+		"[ V21.B16 ]": "[V21.B16]",
+		"V31.B [15]":  "V31.B[15]",
+	}
+	for in, want := range cases {
+		toks := lexOperands(in)
+		if got := renderOps(toks); got != want {
+			t.Errorf("renderOps(%q) = %q, want %q", in, got, want)
+		}
+	}
+}
+
+// TestSIMDBracketRoundTrip formats whole functions carrying the bracket
+// shapes of the arm64 vector kernels and pins the output byte for byte.  The
+// brackets are load-bearing: formatting must not change what the file
+// assembles to, so the formatted text keeps every bracket, re-formats to
+// itself and still parses cleanly.
+func TestSIMDBracketRoundTrip(t *testing.T) {
+	in := "#include \"textflag.h\"\n" +
+		"\n" +
+		"TEXT ·f(SB), NOSPLIT, $0\n" +
+		"VDUP V31.B[15], R3\n" +
+		"VTBL V22.B16, [V28.B16], V11.B16\n" +
+		"VLD1 (R2), [V21.B16]\n" +
+		"VMOVQ $0x70, $0x80, V10\n" +
+		"RET\n"
+
+	want := "#include \"textflag.h\"\n" +
+		"\n" +
+		"TEXT ·f(SB), NOSPLIT, $0\n" +
+		"\tVDUP  V31.B[15], R3\n" +
+		"\tVTBL  V22.B16, [V28.B16], V11.B16\n" +
+		"\tVLD1  (R2), [V21.B16]\n" +
+		"\tVMOVQ $0x70, $0x80, V10\n" +
+		"\tRET\n"
+
+	got := Source(in)
+	if got != want {
+		t.Fatalf("formatting mismatch:\n--- got ---\n%q\n--- want ---\n%q", got, want)
+	}
+	if again := Source(got); again != got {
+		t.Fatalf("not idempotent:\n%q", again)
+	}
+	if n := strings.Count(got, "["); n != 3 {
+		t.Errorf("output carries %d '[', want 3:\n%s", n, got)
+	}
+	if _, errs := parser.Parse("in.s", got); len(errs) > 0 {
+		t.Errorf("formatted output no longer parses: %v", errs)
+	}
+}
+
+// TestBracketFormsRoundTrip runs every bracket shape of the vector kernels
+// through a full format pass as its own single-instruction function, where
+// the canonical form is the line itself indented: formatting must be a no-op
+// on each, so no bracket moves, vanishes or gains a space.
+func TestBracketFormsRoundTrip(t *testing.T) {
+	for _, instr := range []string{
+		"VDUP V31.B[15], V18",
+		"VDUP V19.S[3], V18.S4",
+		"VDUP V1.D[1], V2.D2",
+		"VMOV V13.S[0], R20",
+		"VMOV V11.B[11], V16.B[12]",
+		"VMOV R20, V21.B[2]",
+		"VTBL V22.B16, [V28.B16], V11.B16",
+		"VTBL V18.B8, [V17.B16, V18.B16], V22.B8",
+		"VTBL V31.B8, [V14.B16, V15.B16, V16.B16, V17.B16], V15.B8",
+		"VLD1 (R2), [V21.B16]",
+		"VLD1 (R24), [V18.D1, V19.D1, V20.D1]",
+		"VLD1 (R29), [V14.D1, V15.D1, V16.D1, V17.D1]",
+		"VLD1.P 32(R1), [V2.B16, V3.B16]",
+		"VLD1R (R1), [V9.B8]",
+		"VLD4R (R0), [V0.B8, V1.B8, V2.B8, V3.B8]",
+		"VST1 [V2.S4, V3.S4, V4.S4, V5.S4], (R14)",
+		"VST1.P [V2.B16], (R1)",
+		"VST1.P [V2.B16, V3.B16], 32(R1)",
+		"VMOVQ $0x70, $0x80, V10",
+	} {
+		src := "TEXT ·f(SB), NOSPLIT, $0\n" + instr + "\nRET\n"
+		want := "TEXT ·f(SB), NOSPLIT, $0\n\t" + instr + "\n\tRET\n"
+		got := Source(src)
+		if got != want {
+			t.Errorf("formatting %q:\n got  %q\n want %q", instr, got, want)
+			continue
+		}
+		if again := Source(got); again != got {
+			t.Errorf("not idempotent for %q:\n%q", instr, again)
+		}
+		if _, errs := parser.Parse("in.s", got); len(errs) > 0 {
+			t.Errorf("formatted output of %q no longer parses: %v", instr, errs)
+		}
+	}
+}
+
 // TestFlagListRoundTrip pins the '|' flag separator and the <ABIInternal>
 // marker through a full format pass: the bars the Go toolchain requires and
 // the ABI bracket must survive byte for byte, on TEXT and GLOBL alike.
@@ -76,16 +76,16 @@ TEXT ·simdTable(SB), NOSPLIT, $0-0

 // func simdLoadStore()
 TEXT ·simdLoadStore(SB), NOSPLIT, $0-0
-	VLD1 (R2), [V21.B16]
-	VLD1 (R24), [V18.D1, V19.D1, V20.D1]
-	VLD1 (R29), [V14.D1, V15.D1, V16.D1, V17.D1]
+	VLD1   (R2), [V21.B16]
+	VLD1   (R24), [V18.D1, V19.D1, V20.D1]
+	VLD1   (R29), [V14.D1, V15.D1, V16.D1, V17.D1]
 	VLD1.P 32(R1), [V2.B16, V3.B16]
 	VLD1.P 64(R4), [V5.B16, V6.B16, V7.B16, V8.B16]
-	VLD1R (R1), [V9.B8]
-	VLD1R (R0), [V0.B16]
-	VLD4R (R0), [V0.B8, V1.B8, V2.B8, V3.B8]
-	VST1 [V2.S4, V3.S4, V4.S4, V5.S4], (R14)
-	VST1 [V14.H4, V15.H4, V16.H4], (R27)
+	VLD1R  (R1), [V9.B8]
+	VLD1R  (R0), [V0.B16]
+	VLD4R  (R0), [V0.B8, V1.B8, V2.B8, V3.B8]
+	VST1   [V2.S4, V3.S4, V4.S4, V5.S4], (R14)
+	VST1   [V14.H4, V15.H4, V16.H4], (R27)
 	VST1.P [V2.B16], (R1)
 	VST1.P [V2.B16, V3.B16], 32(R1)
 	RET