Skip to content

Commit 0e466a8

Browse files
committed
cmd/compile: modify float-to-[u]int so that amd64 and arm64 match
Eventual goal is that all the architectures agree, and are sensible. The test will be build-tagged to exclude not-yet-handled platforms. This change also bisects the conversion change in case of bugs. (`bisect -compile=convert ...`) Change-Id: I98528666b0a3fde17cbe8d69b612d01da18dce85 Reviewed-on: https://go-review.googlesource.com/c/go/+/691135 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Keith Randall <[email protected]> Reviewed-by: Keith Randall <[email protected]>
1 parent 4837fbe commit 0e466a8

File tree

8 files changed

+569
-27
lines changed

8 files changed

+569
-27
lines changed

src/cmd/compile/internal/base/debug.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ type DebugFlags struct {
2020
Append int `help:"print information about append compilation"`
2121
Checkptr int `help:"instrument unsafe pointer conversions\n0: instrumentation disabled\n1: conversions involving unsafe.Pointer are instrumented\n2: conversions to unsafe.Pointer force heap allocation" concurrent:"ok"`
2222
Closure int `help:"print information about closure compilation"`
23+
Converthash string `help:"hash value for use in debugging changes to platform-dependent float-to-[u]int conversion" concurrent:"ok"`
2324
Defer int `help:"print information about defer compilation"`
2425
DisableNil int `help:"disable nil checks" concurrent:"ok"`
2526
DumpInlFuncProps string `help:"dump function properties from inl heuristics to specified file"`

src/cmd/compile/internal/base/flag.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,9 @@ func ParseFlags() {
262262
Debug.LoopVar = 1
263263
}
264264

265+
if Debug.Converthash != "" {
266+
ConvertHash = NewHashDebug("converthash", Debug.Converthash, nil)
267+
}
265268
if Debug.Fmahash != "" {
266269
FmaHash = NewHashDebug("fmahash", Debug.Fmahash, nil)
267270
}

src/cmd/compile/internal/base/hashdebug.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ func (d *HashDebug) SetInlineSuffixOnly(b bool) *HashDebug {
5353
// The default compiler-debugging HashDebug, for "-d=gossahash=..."
5454
var hashDebug *HashDebug
5555

56+
var ConvertHash *HashDebug // for debugging float-to-[u]int conversion changes
5657
var FmaHash *HashDebug // for debugging fused-multiply-add floating point changes
5758
var LoopVarHash *HashDebug // for debugging shared/private loop variable changes
5859
var PGOHash *HashDebug // for debugging PGO optimization decisions

src/cmd/compile/internal/ssa/_gen/AMD64.rules

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -162,10 +162,19 @@
162162
(Cvt64to32F ...) => (CVTSQ2SS ...)
163163
(Cvt64to64F ...) => (CVTSQ2SD ...)
164164

165-
(Cvt32Fto32 ...) => (CVTTSS2SL ...)
166-
(Cvt32Fto64 ...) => (CVTTSS2SQ ...)
167-
(Cvt64Fto32 ...) => (CVTTSD2SL ...)
168-
(Cvt64Fto64 ...) => (CVTTSD2SQ ...)
165+
// Float, to int.
166+
// To make AMD64 "overflow" return max positive instead of max negative, compute
167+
// y and not x, smear the sign bit, and xor.
168+
(Cvt32Fto32 <t> x) && base.ConvertHash.MatchPos(v.Pos, nil) => (XORL <t> y (SARLconst <t> [31] (ANDL <t> y:(CVTTSS2SL <t> x) (NOTL <typ.Int32> (MOVLf2i x)))))
169+
(Cvt64Fto32 <t> x) && base.ConvertHash.MatchPos(v.Pos, nil) => (XORL <t> y (SARLconst <t> [31] (ANDL <t> y:(CVTTSD2SL <t> x) (NOTL <typ.Int32> (MOVLf2i (CVTSD2SS <typ.Float32> x))))))
170+
171+
(Cvt32Fto64 <t> x) && base.ConvertHash.MatchPos(v.Pos, nil) => (XORQ <t> y (SARQconst <t> [63] (ANDQ <t> y:(CVTTSS2SQ <t> x) (NOTQ <typ.Int64> (MOVQf2i (CVTSS2SD <typ.Float64> x))) )))
172+
(Cvt64Fto64 <t> x) && base.ConvertHash.MatchPos(v.Pos, nil) => (XORQ <t> y (SARQconst <t> [63] (ANDQ <t> y:(CVTTSD2SQ <t> x) (NOTQ <typ.Int64> (MOVQf2i x)))))
173+
174+
(Cvt32Fto32 <t> x) && !base.ConvertHash.MatchPos(v.Pos, nil) => (CVTTSS2SL <t> x)
175+
(Cvt32Fto64 <t> x) && !base.ConvertHash.MatchPos(v.Pos, nil) => (CVTTSS2SQ <t> x)
176+
(Cvt64Fto32 <t> x) && !base.ConvertHash.MatchPos(v.Pos, nil) => (CVTTSD2SL <t> x)
177+
(Cvt64Fto64 <t> x) && !base.ConvertHash.MatchPos(v.Pos, nil) => (CVTTSD2SQ <t> x)
169178

170179
(Cvt32Fto64F ...) => (CVTSS2SD ...)
171180
(Cvt64Fto32F ...) => (CVTSD2SS ...)

src/cmd/compile/internal/ssa/rewriteAMD64.go

Lines changed: 189 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)