From 0d789c8353d99caf1199e876cc4fdf7c8d38f0a9 Mon Sep 17 00:00:00 2001 From: Michael McLoughlin Date: Sun, 11 Jun 2023 16:12:59 -0700 Subject: [PATCH] operand: fix integer float data (#393) Issue #387 pointed out that integer float data is printed incorrectly, such that it is not parsed correctly by the Go assembler. Specifically, integer values need the decimal point, otherwise they will be treated as integers. For example, 1 must be represented as `$(1.)` or `$(1.0)` to be parsed correctly. This PR fixes that problem and adds a regression test. The root of the problem was that the formatting verb `%#v` does not have the right behavior for integers. We fix it by deferring to custom `String()` function for the float operand types. Fixes #387 Closes #388 --- operand/const.go | 30 ++++++++++++- operand/const_test.go | 2 + operand/make_const.go | 3 +- operand/zconst.go | 4 +- tests/fixedbugs/issue387/asm.go | 53 +++++++++++++++++++++++ tests/fixedbugs/issue387/doc.go | 3 ++ tests/fixedbugs/issue387/issue387.s | 45 +++++++++++++++++++ tests/fixedbugs/issue387/issue387_test.go | 27 ++++++++++++ tests/fixedbugs/issue387/stub.go | 9 ++++ 9 files changed, 172 insertions(+), 4 deletions(-) create mode 100644 tests/fixedbugs/issue387/asm.go create mode 100644 tests/fixedbugs/issue387/doc.go create mode 100644 tests/fixedbugs/issue387/issue387.s create mode 100644 tests/fixedbugs/issue387/issue387_test.go create mode 100644 tests/fixedbugs/issue387/stub.go diff --git a/operand/const.go b/operand/const.go index b2c6a6f7..10d57c52 100644 --- a/operand/const.go +++ b/operand/const.go @@ -1,6 +1,10 @@ package operand -import "fmt" +import ( + "fmt" + "strconv" + "strings" +) // Constant represents a constant literal. type Constant interface { @@ -11,6 +15,30 @@ type Constant interface { //go:generate go run make_const.go -output zconst.go +// Special cases for floating point string representation. +// +// Issue 387 pointed out that floating point values that happen to be integers +// need to have a decimal point to be parsed correctly. + +// String returns a representation the 32-bit float which is guaranteed to be +// parsed as a floating point constant by the Go assembler. +func (f F32) String() string { return asmfloat(float64(f), 32) } + +// String returns a representation the 64-bit float which is guaranteed to be +// parsed as a floating point constant by the Go assembler. +func (f F64) String() string { return asmfloat(float64(f), 64) } + +// asmfloat represents x as a string such that the assembler scanner will always +// recognize it as a float. Specifically, ensure that when x is an integral +// value, the result will still have a decimal point. +func asmfloat(x float64, bits int) string { + s := strconv.FormatFloat(x, 'f', -1, bits) + if !strings.ContainsRune(s, '.') { + s += ".0" + } + return s +} + // String is a string constant. type String string diff --git a/operand/const_test.go b/operand/const_test.go index aa4593f4..d200d019 100644 --- a/operand/const_test.go +++ b/operand/const_test.go @@ -9,7 +9,9 @@ func TestConstants(t *testing.T) { Bytes int }{ {F32(3.1415), "$(3.1415)", 4}, + {F32(42), "$(42.0)", 4}, {F64(3.1415), "$(3.1415)", 8}, + {F64(42), "$(42.0)", 8}, {U8(42), "$0x2a", 1}, {U16(42), "$0x002a", 2}, {U32(42), "$0x0000002a", 4}, diff --git a/operand/make_const.go b/operand/make_const.go index be968c5f..36b467bf 100644 --- a/operand/make_const.go +++ b/operand/make_const.go @@ -38,7 +38,8 @@ func PrintConstTypes(w io.Writer) { bs := strconv.Itoa(bits) if n >= 4 { - PrintConstType(w, "F"+bs, "float"+bs, "(%#v)", n, fmt.Sprintf("F%d is a %d-bit floating point constant.", bits, bits)) + // Use string format verb to direct to our custom implementation. + PrintConstType(w, "F"+bs, "float"+bs, "(%s)", n, fmt.Sprintf("F%d is a %d-bit floating point constant.", bits, bits)) } PrintConstType(w, "I"+bs, "int"+bs, "%+d", n, fmt.Sprintf("I%d is a %d-bit signed integer constant.", bits, bits)) PrintConstType(w, "U"+bs, "uint"+bs, "%#0"+strconv.Itoa(2*n)+"x", n, fmt.Sprintf("U%d is a %d-bit unsigned integer constant.", bits, bits)) diff --git a/operand/zconst.go b/operand/zconst.go index 324b4a96..14b128a1 100644 --- a/operand/zconst.go +++ b/operand/zconst.go @@ -35,7 +35,7 @@ func (u U16) constant() {} // F32 is a 32-bit floating point constant. type F32 float32 -func (f F32) Asm() string { return fmt.Sprintf("$(%#v)", f) } +func (f F32) Asm() string { return fmt.Sprintf("$(%s)", f) } func (f F32) Bytes() int { return 4 } func (f F32) constant() {} @@ -56,7 +56,7 @@ func (u U32) constant() {} // F64 is a 64-bit floating point constant. type F64 float64 -func (f F64) Asm() string { return fmt.Sprintf("$(%#v)", f) } +func (f F64) Asm() string { return fmt.Sprintf("$(%s)", f) } func (f F64) Bytes() int { return 8 } func (f F64) constant() {} diff --git a/tests/fixedbugs/issue387/asm.go b/tests/fixedbugs/issue387/asm.go new file mode 100644 index 00000000..c4e94b93 --- /dev/null +++ b/tests/fixedbugs/issue387/asm.go @@ -0,0 +1,53 @@ +//go:build ignore +// +build ignore + +package main + +import ( + . "github.com/mmcloughlin/avo/build" + . "github.com/mmcloughlin/avo/operand" +) + +// Float32 generates a function which indexes into an array of single-precision +// integer float values. +func Float32() { + f32 := GLOBL("f32", RODATA|NOPTR) + for i := 0; i < 10; i++ { + DATA(4*i, F32(i)) + } + + TEXT("Float32", NOSPLIT, "func(i int) float32") + Doc("Float32 indexes into an array of single-precision integral floats.") + i := Load(Param("i"), GP64()) + ptr := Mem{Base: GP64()} + LEAQ(f32, ptr.Base) + x := XMM() + MOVSS(ptr.Idx(i, 4), x) + Store(x, ReturnIndex(0)) + RET() +} + +// Float64 generates a function which indexes into an array of double-precision +// integer float values. +func Float64() { + f64 := GLOBL("f64", RODATA|NOPTR) + for i := 0; i < 10; i++ { + DATA(8*i, F64(i)) + } + + TEXT("Float64", NOSPLIT, "func(i int) float64") + Doc("Float64 indexes into an array of double-precision integral floats.") + i := Load(Param("i"), GP64()) + ptr := Mem{Base: GP64()} + LEAQ(f64, ptr.Base) + x := XMM() + MOVSD(ptr.Idx(i, 8), x) + Store(x, ReturnIndex(0)) + RET() +} + +func main() { + Float32() + Float64() + Generate() +} diff --git a/tests/fixedbugs/issue387/doc.go b/tests/fixedbugs/issue387/doc.go new file mode 100644 index 00000000..dd65a393 --- /dev/null +++ b/tests/fixedbugs/issue387/doc.go @@ -0,0 +1,3 @@ +// Package issue387 tests representation of floating point data with integer +// values. +package issue387 diff --git a/tests/fixedbugs/issue387/issue387.s b/tests/fixedbugs/issue387/issue387.s new file mode 100644 index 00000000..8dc1d9dd --- /dev/null +++ b/tests/fixedbugs/issue387/issue387.s @@ -0,0 +1,45 @@ +// Code generated by command: go run asm.go -out issue387.s -stubs stub.go. DO NOT EDIT. + +#include "textflag.h" + +DATA f32<>+0(SB)/4, $(0.0) +DATA f32<>+4(SB)/4, $(1.0) +DATA f32<>+8(SB)/4, $(2.0) +DATA f32<>+12(SB)/4, $(3.0) +DATA f32<>+16(SB)/4, $(4.0) +DATA f32<>+20(SB)/4, $(5.0) +DATA f32<>+24(SB)/4, $(6.0) +DATA f32<>+28(SB)/4, $(7.0) +DATA f32<>+32(SB)/4, $(8.0) +DATA f32<>+36(SB)/4, $(9.0) +GLOBL f32<>(SB), RODATA|NOPTR, $40 + +// func Float32(i int) float32 +// Requires: SSE +TEXT ·Float32(SB), NOSPLIT, $0-12 + MOVQ i+0(FP), AX + LEAQ f32<>+0(SB), CX + MOVSS (CX)(AX*4), X0 + MOVSS X0, ret+8(FP) + RET + +DATA f64<>+0(SB)/8, $(0.0) +DATA f64<>+8(SB)/8, $(1.0) +DATA f64<>+16(SB)/8, $(2.0) +DATA f64<>+24(SB)/8, $(3.0) +DATA f64<>+32(SB)/8, $(4.0) +DATA f64<>+40(SB)/8, $(5.0) +DATA f64<>+48(SB)/8, $(6.0) +DATA f64<>+56(SB)/8, $(7.0) +DATA f64<>+64(SB)/8, $(8.0) +DATA f64<>+72(SB)/8, $(9.0) +GLOBL f64<>(SB), RODATA|NOPTR, $80 + +// func Float64(i int) float64 +// Requires: SSE2 +TEXT ·Float64(SB), NOSPLIT, $0-16 + MOVQ i+0(FP), AX + LEAQ f64<>+0(SB), CX + MOVSD (CX)(AX*8), X0 + MOVSD X0, ret+8(FP) + RET diff --git a/tests/fixedbugs/issue387/issue387_test.go b/tests/fixedbugs/issue387/issue387_test.go new file mode 100644 index 00000000..545a827f --- /dev/null +++ b/tests/fixedbugs/issue387/issue387_test.go @@ -0,0 +1,27 @@ +package issue387 + +import ( + "testing" +) + +//go:generate go run asm.go -out issue387.s -stubs stub.go + +func TestFloat32(t *testing.T) { + for i := 0; i < 10; i++ { + got := Float32(i) + expect := float32(i) + if got != expect { + t.Fatalf("Float32(%d) = %#v; expect %#v", i, got, expect) + } + } +} + +func TestFloat64(t *testing.T) { + for i := 0; i < 10; i++ { + got := Float64(i) + expect := float64(i) + if got != expect { + t.Fatalf("Float64(%d) = %#v; expect %#v", i, got, expect) + } + } +} diff --git a/tests/fixedbugs/issue387/stub.go b/tests/fixedbugs/issue387/stub.go new file mode 100644 index 00000000..1f101831 --- /dev/null +++ b/tests/fixedbugs/issue387/stub.go @@ -0,0 +1,9 @@ +// Code generated by command: go run asm.go -out issue387.s -stubs stub.go. DO NOT EDIT. + +package issue387 + +// Float32 indexes into an array of single-precision integral floats. +func Float32(i int) float32 + +// Float64 indexes into an array of double-precision integral floats. +func Float64(i int) float64