cmd/compile: stop using fucomi* ops for 387 builds

The fucomi* opcodes were only introduced for the Pentium Pro.
They do not exist for an MMX Pentium.  Use the fucom* instructions
instead and move the condition codes from the fp flags register to
the integer flags register explicitly.

The use of fucomi* opcodes in ggen.go was introduced in 1.5 (CL 8738).
The bad ops were generated for 64-bit floating-point comparisons.

The use of fucomi* opcodes in gsubr.go dates back to at least 1.1.
The bad ops were generated for float{32,64} to uint64 conversions.

Fixes #13923

Change-Id: I5290599f5edea8abf8fb18036f44fa78bd1fc9e6
Reviewed-on: https://go-review.googlesource.com/18590
Reviewed-by: Minux Ma <minux@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>

diff --git a/src/cmd/compile/internal/gc/float_test.go b/src/cmd/compile/internal/gc/float_test.go
new file mode 100644 (file)
index 0000000..c761e96
--- /dev/null
+++ b/src/cmd/compile/internal/gc/float_test.go
@@ -0,0 +1,102 @@
+// Copyright 2016 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package gc
+
+import "testing"
+
+// For GO386=387, make sure fucomi* opcodes are not used
+// for comparison operations.
+// Note that this test will fail only on a Pentium MMX
+// processor (with GOARCH=386 GO386=387), as it just runs
+// some code and looks for an unimplemented instruction fault.
+
+//go:noinline
+func compare1(a, b float64) bool {
+       return a < b
+}
+
+//go:noinline
+func compare2(a, b float32) bool {
+       return a < b
+}
+
+func TestFloatCompare(t *testing.T) {
+       if !compare1(3, 5) {
+               t.Errorf("compare1 returned false")
+       }
+       if !compare2(3, 5) {
+               t.Errorf("compare2 returned false")
+       }
+}
+
+// For GO386=387, make sure fucomi* opcodes are not used
+// for float->int conversions.
+
+//go:noinline
+func cvt1(a float64) uint64 {
+       return uint64(a)
+}
+
+//go:noinline
+func cvt2(a float64) uint32 {
+       return uint32(a)
+}
+
+//go:noinline
+func cvt3(a float32) uint64 {
+       return uint64(a)
+}
+
+//go:noinline
+func cvt4(a float32) uint32 {
+       return uint32(a)
+}
+
+//go:noinline
+func cvt5(a float64) int64 {
+       return int64(a)
+}
+
+//go:noinline
+func cvt6(a float64) int32 {
+       return int32(a)
+}
+
+//go:noinline
+func cvt7(a float32) int64 {
+       return int64(a)
+}
+
+//go:noinline
+func cvt8(a float32) int32 {
+       return int32(a)
+}
+
+func TestFloatConvert(t *testing.T) {
+       if got := cvt1(3.5); got != 3 {
+               t.Errorf("cvt1 got %d, wanted 3", got)
+       }
+       if got := cvt2(3.5); got != 3 {
+               t.Errorf("cvt2 got %d, wanted 3", got)
+       }
+       if got := cvt3(3.5); got != 3 {
+               t.Errorf("cvt3 got %d, wanted 3", got)
+       }
+       if got := cvt4(3.5); got != 3 {
+               t.Errorf("cvt4 got %d, wanted 3", got)
+       }
+       if got := cvt5(3.5); got != 3 {
+               t.Errorf("cvt5 got %d, wanted 3", got)
+       }
+       if got := cvt6(3.5); got != 3 {
+               t.Errorf("cvt6 got %d, wanted 3", got)
+       }
+       if got := cvt7(3.5); got != 3 {
+               t.Errorf("cvt7 got %d, wanted 3", got)
+       }
+       if got := cvt8(3.5); got != 3 {
+               t.Errorf("cvt8 got %d, wanted 3", got)
+       }
+}

diff --git a/src/cmd/compile/internal/x86/ggen.go b/src/cmd/compile/internal/x86/ggen.go
index e559a9f5da9dcbb76edc44295135071e76f6e4f9..139b199b5788b6a85e28b46aaacc1501a08a3d11 100644 (file)
--- a/src/cmd/compile/internal/x86/ggen.go
+++ b/src/cmd/compile/internal/x86/ggen.go
@@ -764,9 +764,7 @@ func bgen_float(n *gc.Node, wantTrue bool, likely int, to *obj.Prog) {
                                gc.Cgen(nr, &tmp)
                                gc.Cgen(nl, &tmp)
                        }
-
-                       gins(x86.AFUCOMIP, &tmp, &n2)
-                       gins(x86.AFMOVDP, &tmp, &tmp) // annoying pop but still better than STSW+SAHF
+                       gins(x86.AFUCOMPP, &tmp, &n2)
                } else {
                        // TODO(rsc): The moves back and forth to memory
                        // here are for truncating the value to 32 bits.
@@ -783,9 +781,9 @@ func bgen_float(n *gc.Node, wantTrue bool, likely int, to *obj.Prog) {
                        gc.Cgen(nl, &t2)
                        gmove(&t2, &tmp)
                        gins(x86.AFCOMFP, &t1, &tmp)
-                       gins(x86.AFSTSW, nil, &ax)
-                       gins(x86.ASAHF, nil, nil)
                }
+               gins(x86.AFSTSW, nil, &ax)
+               gins(x86.ASAHF, nil, nil)
        } else {
                // Not 387
                if !nl.Addable {

diff --git a/src/cmd/compile/internal/x86/gsubr.go b/src/cmd/compile/internal/x86/gsubr.go
index 03978578b7575a0addef14fda2451138fa61978c..98595716cfa3935f8e5f289592eb17f57d4aca27 100644 (file)
--- a/src/cmd/compile/internal/x86/gsubr.go
+++ b/src/cmd/compile/internal/x86/gsubr.go
@@ -1198,14 +1198,17 @@ func floatmove(f *gc.Node, t *gc.Node) {
 
                // if 0 > v { answer = 0 }
                gins(x86.AFMOVD, &zerof, &f0)
-
-               gins(x86.AFUCOMIP, &f0, &f1)
+               gins(x86.AFUCOMP, &f0, &f1)
+               gins(x86.AFSTSW, nil, &ax)
+               gins(x86.ASAHF, nil, nil)
                p1 := gc.Gbranch(optoas(gc.OGT, gc.Types[tt]), nil, 0)
 
                // if 1<<64 <= v { answer = 0 too }
                gins(x86.AFMOVD, &two64f, &f0)
 
-               gins(x86.AFUCOMIP, &f0, &f1)
+               gins(x86.AFUCOMP, &f0, &f1)
+               gins(x86.AFSTSW, nil, &ax)
+               gins(x86.ASAHF, nil, nil)
                p2 := gc.Gbranch(optoas(gc.OGT, gc.Types[tt]), nil, 0)
                gc.Patch(p1, gc.Pc)
                gins(x86.AFMOVVP, &f0, t) // don't care about t, but will pop the stack
@@ -1235,7 +1238,9 @@ func floatmove(f *gc.Node, t *gc.Node) {
                // actual work
                gins(x86.AFMOVD, &two63f, &f0)
 
-               gins(x86.AFUCOMIP, &f0, &f1)
+               gins(x86.AFUCOMP, &f0, &f1)
+               gins(x86.AFSTSW, nil, &ax)
+               gins(x86.ASAHF, nil, nil)
                p2 = gc.Gbranch(optoas(gc.OLE, gc.Types[tt]), nil, 0)
                gins(x86.AFMOVVP, &f0, t)
                p3 := gc.Gbranch(obj.AJMP, nil, 0)

diff --git a/src/cmd/compile/internal/x86/prog.go b/src/cmd/compile/internal/x86/prog.go
index 5ff7bb8e8c95cb21b6cf92c834ca07ffb9927c67..22ee23db12c1032f6f2b651c389a027287d75f88 100644 (file)
--- a/src/cmd/compile/internal/x86/prog.go
+++ b/src/cmd/compile/internal/x86/prog.go
@@ -91,8 +91,12 @@ var progtable = [x86.ALAST]obj.ProgInfo{
        x86.AFCOMDPP:   {Flags: gc.SizeD | gc.LeftAddr | gc.RightRead},
        x86.AFCOMF:     {Flags: gc.SizeF | gc.LeftAddr | gc.RightRead},
        x86.AFCOMFP:    {Flags: gc.SizeF | gc.LeftAddr | gc.RightRead},
-       x86.AFUCOMIP:   {Flags: gc.SizeF | gc.LeftAddr | gc.RightRead},
-       x86.AFCHS:      {Flags: gc.SizeD | RightRdwr}, // also SizeF
+       // NOTE(khr): don't use FUCOMI* instructions, not available
+       // on Pentium MMX.  See issue 13923.
+       //x86.AFUCOMIP:   {Flags: gc.SizeF | gc.LeftAddr | gc.RightRead},
+       x86.AFUCOMP:  {Flags: gc.SizeD | gc.LeftRead | gc.RightRead},
+       x86.AFUCOMPP: {Flags: gc.SizeD | gc.LeftRead | gc.RightRead},
+       x86.AFCHS:    {Flags: gc.SizeD | RightRdwr}, // also SizeF
 
        x86.AFDIVDP:  {Flags: gc.SizeD | gc.LeftAddr | RightRdwr},
        x86.AFDIVF:   {Flags: gc.SizeF | gc.LeftAddr | RightRdwr},