tests/fixedbugs: regression test for issue 100 (#129)

Adds a regression test based on klauspost/compress#186. This necessitated some related changes:

* Mark "RET" as a terminal instruction
* printer refactor to maintain compatibility with asmfmt
* Tweaks to other regression tests to ensure they are run correctly in CI

Updates #100 #65 #8

examples/stadtx/stadtx.s

@@ -33,10 +33,10 @@ TEXT ·Hash(SB), NOSPLIT, $0-40
 	JE    shortCore3
 
 shortCore3:
-	MOVQ  (CX), SI
-	MOVQ  $0x9c1b8e1e9628323f, DI
-	IMULQ DI, SI
-	ADDQ  SI, BX
+	MOVQ  (CX), AX
+	MOVQ  $0x9c1b8e1e9628323f, SI
+	IMULQ SI, AX
+	ADDQ  AX, BX
 	RORQ  $0x11, BX
 	XORQ  BP, BX
 	RORQ  $0x35, BP
@@ -45,10 +45,10 @@ shortCore3:
 	SUBQ  $0x00000008, DX
 
 shortCore2:
-	MOVQ  (CX), SI
-	MOVQ  $0x9c1b8e1e9628323f, DI
-	IMULQ DI, SI
-	ADDQ  SI, BX
+	MOVQ  (CX), AX
+	MOVQ  $0x9c1b8e1e9628323f, SI
+	IMULQ SI, AX
+	ADDQ  AX, BX
 	RORQ  $0x11, BX
 	XORQ  BP, BX
 	RORQ  $0x35, BP
@@ -57,10 +57,10 @@ shortCore2:
 	SUBQ  $0x00000008, DX
 
 shortCore1:
-	MOVQ  (CX), SI
-	MOVQ  $0x9c1b8e1e9628323f, DI
-	IMULQ DI, SI
-	ADDQ  SI, BX
+	MOVQ  (CX), AX
+	MOVQ  $0x9c1b8e1e9628323f, SI
+	IMULQ SI, AX
+	ADDQ  AX, BX
 	RORQ  $0x11, BX
 	XORQ  BP, BX
 	RORQ  $0x35, BP
@@ -87,38 +87,38 @@ shortCore0:
 	JE   shortTail7
 
 shortTail7:
-	MOVBQZX 6(CX), SI
-	SHLQ    $0x20, SI
-	ADDQ    SI, BX
+	MOVBQZX 6(CX), DX
+	SHLQ    $0x20, DX
+	ADDQ    DX, BX
 
 shortTail6:
-	MOVBQZX 5(CX), SI
-	SHLQ    $0x30, SI
-	ADDQ    SI, BP
+	MOVBQZX 5(CX), DX
+	SHLQ    $0x30, DX
+	ADDQ    DX, BP
 
 shortTail5:
-	MOVBQZX 4(CX), SI
-	SHLQ    $0x10, SI
-	ADDQ    SI, BX
+	MOVBQZX 4(CX), DX
+	SHLQ    $0x10, DX
+	ADDQ    DX, BX
 
 shortTail4:
-	MOVLQZX (CX), SI
-	ADDQ    SI, BP
+	MOVLQZX (CX), DX
+	ADDQ    DX, BP
 	JMP     shortAfter
 
 shortTail3:
-	MOVBQZX 2(CX), SI
-	SHLQ    $0x30, SI
-	ADDQ    SI, BX
+	MOVBQZX 2(CX), DX
+	SHLQ    $0x30, DX
+	ADDQ    DX, BX
 
 shortTail2:
-	MOVWQZX (CX), SI
-	ADDQ    SI, BP
+	MOVWQZX (CX), DX
+	ADDQ    DX, BP
 	JMP     shortAfter
 
 shortTail1:
-	MOVBQZX (CX), SI
-	ADDQ    SI, BX
+	MOVBQZX (CX), DX
+	ADDQ    DX, BX
 
 shortTail0:
 	RORQ $0x20, BP
@@ -262,37 +262,37 @@ longCore0:
 	JE    longTail7
 
 longTail7:
-	MOVBQZX 6(CX), SI
-	ADDQ    SI, BP
+	MOVBQZX 6(CX), DX
+	ADDQ    DX, BP
 
 longTail6:
-	MOVWQZX 4(CX), SI
-	ADDQ    SI, DI
-	MOVLQZX (CX), SI
-	ADDQ    SI, AX
+	MOVWQZX 4(CX), DX
+	ADDQ    DX, DI
+	MOVLQZX (CX), DX
+	ADDQ    DX, AX
 	JMP     longAfter
 
 longTail5:
-	MOVBQZX 4(CX), SI
-	ADDQ    SI, BP
+	MOVBQZX 4(CX), DX
+	ADDQ    DX, BP
 
 longTail4:
-	MOVLQZX (CX), SI
-	ADDQ    SI, DI
+	MOVLQZX (CX), DX
+	ADDQ    DX, DI
 	JMP     longAfter
 
 longTail3:
-	MOVBQZX 2(CX), SI
-	ADDQ    SI, AX
+	MOVBQZX 2(CX), DX
+	ADDQ    DX, AX
 
 longTail2:
-	MOVWQZX (CX), SI
-	ADDQ    SI, BP
+	MOVWQZX (CX), DX
+	ADDQ    DX, BP
 	JMP     longAfter
 
 longTail1:
-	MOVBQZX (CX), SI
-	ADDQ    SI, DI
+	MOVBQZX (CX), DX
+	ADDQ    DX, DI
 
 longTail0:
 	ROLQ $0x20, AX

internal/gen/ctors.go

@@ -98,6 +98,10 @@ func construct(i inst.Instruction, f inst.Form, s signature) string {
 	}
 
 	// Branch variables.
+	if i.IsTerminal() {
+		fmt.Fprintf(buf, "\tIsTerminal: true,\n")
+	}
+
 	if i.IsBranch() {
 		fmt.Fprintf(buf, "\tIsBranch: true,\n")
 		fmt.Fprintf(buf, "\tIsConditional: %#v,\n", i.IsConditionalBranch())

ir/ir.go

@@ -61,6 +61,11 @@ type Instruction struct {
 
 func (i *Instruction) node() {}
 
+// IsUnconditionalBranch reports whether i is an unconditional branch.
+func (i Instruction) IsUnconditionalBranch() bool {
+	return i.IsBranch && !i.IsConditional
+}
+
 // TargetLabel returns the label referenced by this instruction. Returns nil if
 // no label is referenced.
 func (i Instruction) TargetLabel() *Label {

pass/cfg.go

@@ -62,7 +62,7 @@ func CFG(fn *ir.Function) error {
 		// Otherwise, could continue to the following instruction.
 		switch {
 		case cur.IsTerminal:
-		case cur.IsBranch && !cur.IsConditional:
+		case cur.IsUnconditionalBranch():
 		default:
 			cur.Succ = append(cur.Succ, nxt)
 		}

printer/goasm.go

@@ -1,10 +1,8 @@
 package printer
 
 import (
-	"fmt"
 	"strconv"
 	"strings"
-	"text/tabwriter"
 
 	"github.com/mmcloughlin/avo/internal/prnt"
 	"github.com/mmcloughlin/avo/ir"
@@ -17,6 +15,9 @@ const dot = "\u00b7"
 type goasm struct {
 	cfg Config
 	prnt.Generator
+
+	instructions []*ir.Instruction
+	clear        bool
 }
 
 // NewGoAsm constructs a printer for writing Go assembly files.
@@ -87,43 +88,67 @@ func (p *goasm) function(f *ir.Function) {
 	}
 	p.Printf(", %s\n", textsize(f))
 
-	w := p.tabwriter()
-	clear := true
-	flush := func() {
-		w.Flush()
-		w = p.tabwriter()
-		if !clear {
-			p.NL()
-			clear = true
-		}
-	}
+	p.clear = true
 	for _, node := range f.Nodes {
 		switch n := node.(type) {
 		case *ir.Instruction:
-			leader := []byte{tabwriter.Escape, '\t', tabwriter.Escape}
-			fmt.Fprint(w, string(leader)+n.Opcode)
-			if len(n.Operands) > 0 {
-				fmt.Fprintf(w, "\t%s", joinOperands(n.Operands))
+			p.instruction(n)
+			if n.IsTerminal || n.IsUnconditionalBranch() {
+				p.flush()
 			}
-			fmt.Fprint(w, "\n")
-			clear = false
 		case ir.Label:
-			flush()
+			p.flush()
+			p.ensureclear()
 			p.Printf("%s:\n", n)
 		case *ir.Comment:
-			flush()
+			p.flush()
+			p.ensureclear()
 			for _, line := range n.Lines {
 				p.Printf("\t// %s\n", line)
 			}
 		default:
 			panic("unexpected node type")
 		}
 	}
-	w.Flush()
+	p.flush()
+}
+
+func (p *goasm) instruction(i *ir.Instruction) {
+	p.instructions = append(p.instructions, i)
+	p.clear = false
 }
 
-func (p *goasm) tabwriter() *tabwriter.Writer {
-	return tabwriter.NewWriter(p.Raw(), 4, 4, 1, ' ', tabwriter.StripEscape)
+func (p *goasm) flush() {
+	if len(p.instructions) == 0 {
+		return
+	}
+
+	// Determine instruction width. Instructions with no operands are not
+	// considered in this calculation.
+	width := 0
+	for _, i := range p.instructions {
+		if len(i.Operands) > 0 && len(i.Opcode) > width {
+			width = len(i.Opcode)
+		}
+	}
+
+	// Output instruction block.
+	for _, i := range p.instructions {
+		if len(i.Operands) > 0 {
+			p.Printf("\t%-*s%s\n", width+1, i.Opcode, joinOperands(i.Operands))
+		} else {
+			p.Printf("\t%s\n", i.Opcode)
+		}
+	}
+
+	p.instructions = nil
+}
+
+func (p *goasm) ensureclear() {
+	if !p.clear {
+		p.NL()
+		p.clear = true
+	}
 }
 
 func (p *goasm) global(g *ir.Global) {

printer/goasm_test.go

@@ -81,3 +81,27 @@ func TestConstraints(t *testing.T) {
 		"",
 	})
 }
+
+func TestAlignmentNoOperands(t *testing.T) {
+	ctx := build.NewContext()
+	ctx.Function("alignment")
+	ctx.SignatureExpr("func()")
+	ctx.ADDQ(reg.RAX, reg.RBX)
+	ctx.VMOVDQU(reg.Y4, reg.Y11)
+	ctx.VZEROUPPER()
+	ctx.ADDQ(reg.R9, reg.R13)
+	ctx.RET()
+
+	AssertPrintsLines(t, ctx, printer.NewGoAsm, []string{
+		"// Code generated by avo. DO NOT EDIT.",
+		"",
+		"// func alignment()",
+		"TEXT ·alignment(SB), $0",
+		"\tADDQ    AX, BX",
+		"\tVMOVDQU Y4, Y11",
+		"\tVZEROUPPER",      // instruction with no alignment doesn't affect width
+		"\tADDQ    R9, R13", // retains alignment from above
+		"\tRET",
+		"",
+	})
+}

tests/fixedbugs/issue100/allocfail/LICENSE

@@ -0,0 +1,28 @@
+Copyright (c) 2011 The Snappy-Go Authors. All rights reserved.
+Copyright (c) 2019 Klaus Post. All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions are
+met:
+
+   * Redistributions of source code must retain the above copyright
+notice, this list of conditions and the following disclaimer.
+   * Redistributions in binary form must reproduce the above
+copyright notice, this list of conditions and the following disclaimer
+in the documentation and/or other materials provided with the
+distribution.
+   * Neither the name of Google Inc. nor the names of its
+contributors may be used to endorse or promote products derived from
+this software without specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

tests/fixedbugs/issue100/allocfail/README.md

@@ -0,0 +1,11 @@
+Regression test for [issue
+#100](https://github.com/mmcloughlin/avo/issues/100) based on the original
+reported allocation failure.
+
+Based on the pull request
+[`klauspost/compress#186`](https://github.com/klauspost/compress/pull/186) at
+`c1f3cf132cd8e214b38cc16e418bf2e501ccda93` with the lines after `FIXME`
+comments re-activated and other minimal edits to make it work in this
+environment.
+
+Original code covered by [BSD 3-Clause License](LICENSE).