Skip to content

Commit

Permalink
operand: fix integer float data (#393)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mmcloughlin committed Jun 11, 2023
1 parent 9bef88d commit 0d789c8
Show file tree
Hide file tree
Showing 9 changed files with 172 additions and 4 deletions.
30 changes: 29 additions & 1 deletion operand/const.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package operand

import "fmt"
import (
"fmt"
"strconv"
"strings"
)

// Constant represents a constant literal.
type Constant interface {
Expand All @@ -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

Expand Down
2 changes: 2 additions & 0 deletions operand/const_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
3 changes: 2 additions & 1 deletion operand/make_const.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
4 changes: 2 additions & 2 deletions operand/zconst.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

53 changes: 53 additions & 0 deletions tests/fixedbugs/issue387/asm.go
Original file line number Diff line number Diff line change
@@ -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()
}
3 changes: 3 additions & 0 deletions tests/fixedbugs/issue387/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// Package issue387 tests representation of floating point data with integer
// values.
package issue387
45 changes: 45 additions & 0 deletions tests/fixedbugs/issue387/issue387.s
Original file line number Diff line number Diff line change
@@ -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
27 changes: 27 additions & 0 deletions tests/fixedbugs/issue387/issue387_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
9 changes: 9 additions & 0 deletions tests/fixedbugs/issue387/stub.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 0d789c8

Please sign in to comment.