Stowage/go
2
0
Fork 0
mirror of https://github.com/golang/go.git synced 2025-12-08 06:10:04 +00:00
Code Issues Projects Releases Packages Wiki Activity

cmd/compile: fix wrong complement for arm64 floating-point comparisons

Browse source 
Consider the following example,

  func test(a, b float64, x uint64) uint64 {
    if a < b {
      x = 0
    }
    return x
  }

  func main() {
    fmt.Println(test(1, math.NaN(), 123))
  }

The output is 0, but the expectation is 123.

This is because the rewrite rule

  (CSEL [cc] (MOVDconst [0]) y flag) => (CSEL0 [arm64Negate(cc)] y flag)

converts

  FCMP NaN, 1
  CSEL MI, 0, 123, R0 // if 1 < NaN then R0 = 0 else R0 = 123

to

  FCMP NaN, 1
  CSEL GE, 123, 0, R0 // if 1 >= NaN then R0 = 123 else R0 = 0

But both 1 < NaN and 1 >= NaN are false. So the output is 0, not 123.

The root cause is arm64Negate not handle negation of floating comparison
correctly. According to the ARM manual, the meaning of MI, GE, and PL
are

  MI: Less than
  GE: Greater than or equal to
  PL: Greater than, equal to, or unordered

Because NaN cannot be compared with other numbers, the result of such
comparison is unordered. So when NaN is involved, unlike integer, the
result of !(a < b) is not a >= b, it is a >= b || a is NaN || b is NaN.
This is exactly what PL means. We add NotLessThanF to represent PL. Then
the negation of LessThanF is NotLessThanF rather than GreaterEqualF. The
same reason for the other floating comparison operations.

Fixes #43619

Change-Id: Ia511b0027ad067436bace9fbfd261dbeaae01bcd
Reviewed-on: https://go-review.googlesource.com/c/go/+/283572
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Keith Randall <khr@golang.org>
This commit is contained in:
Junchen Li 2021-01-08 10:20:34 +08:00 • committed by Cherry Zhang
parent c73232d08f
commit d9b79e53bb
5 changed files with 215 additions and 27 deletions
Download patch file Download diff file Expand all files Collapse all files

31
src/cmd/compile/internal/ssa/rewrite.go
View file

@ -974,9 +974,10 @@ func flagArg(v *Value) *Value {
}
// arm64Negate finds the complement to an ARM64 condition code,
// for example Equal -> NotEqual or LessThan -> GreaterEqual
// for example !Equal -> NotEqual or !LessThan -> GreaterEqual
//
// TODO: add floating-point conditions
// For floating point, it's more subtle because NaN is unordered. We do
// !LessThanF -> NotLessThanF, the latter takes care of NaNs.
func arm64Negate(op Op) Op {
switch op {
case OpARM64LessThan:
@ -1000,13 +1001,21 @@ func arm64Negate(op Op) Op {
case OpARM64NotEqual:
return OpARM64Equal
case OpARM64LessThanF:
return OpARM64GreaterEqualF
case OpARM64GreaterThanF:
return OpARM64LessEqualF
return OpARM64NotLessThanF
case OpARM64NotLessThanF:
return OpARM64LessThanF
case OpARM64LessEqualF:
return OpARM64NotLessEqualF
case OpARM64NotLessEqualF:
return OpARM64LessEqualF
case OpARM64GreaterThanF:
return OpARM64NotGreaterThanF
case OpARM64NotGreaterThanF:
return OpARM64GreaterThanF
case OpARM64GreaterEqualF:
return OpARM64LessThanF
return OpARM64NotGreaterEqualF
case OpARM64NotGreaterEqualF:
return OpARM64GreaterEqualF
default:
panic("unreachable")
}
@ -1017,8 +1026,6 @@ func arm64Negate(op Op) Op {
// that the same result would be produced if the arguments
// to the flag-generating instruction were reversed, e.g.
// (InvertFlags (CMP x y)) -> (CMP y x)
//
// TODO: add floating-point conditions
func arm64Invert(op Op) Op {
switch op {
case OpARM64LessThan:
@ -1047,6 +1054,14 @@ func arm64Invert(op Op) Op {
return OpARM64GreaterEqualF
case OpARM64GreaterEqualF:
return OpARM64LessEqualF
case OpARM64NotLessThanF:
return OpARM64NotGreaterThanF
case OpARM64NotGreaterThanF:
return OpARM64NotLessThanF
case OpARM64NotLessEqualF:
return OpARM64NotGreaterEqualF
case OpARM64NotGreaterEqualF:
return OpARM64NotLessEqualF
default:
panic("unreachable")
}