Skip to content

Commit

Permalink
[release-branch.go1.18] cmd/compile: fix If lowering on ARM64
Browse files Browse the repository at this point in the history
On ARM64, an If block is lowered to (NZ cond yes no). This is
incorrect because cond is a boolean value and therefore only the
last byte is meaningful (same as AMD64, see ARM64Ops.go). But here
we are comparing a full register width with 0. Correct it by
comparing only the last bit.

For #52788.
Fixes #53397.

Change-Id: I2cacf9f3d2f45e149c361a290f511b2d4ed845c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/405114
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: David Chase <[email protected]>
Run-TryBot: Cherry Mui <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/go/+/421457
Reviewed-by: Dmitri Shuralyov <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
Run-TryBot: Dmitri Shuralyov <[email protected]>
  • Loading branch information
cherrymui authored and thanm committed Aug 8, 2022
1 parent fcdd099 commit e05bd75
Show file tree
Hide file tree
Showing 3 changed files with 201 additions and 3 deletions.
17 changes: 16 additions & 1 deletion src/cmd/compile/internal/ssa/gen/ARM64.rules
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@
(If (GreaterThanF cc) yes no) => (FGT cc yes no)
(If (GreaterEqualF cc) yes no) => (FGE cc yes no)

(If cond yes no) => (NZ cond yes no)
(If cond yes no) => (TBNZ [0] cond yes no)

// atomic intrinsics
// Note: these ops do not accept offset.
Expand Down Expand Up @@ -593,6 +593,21 @@
(NZ (GreaterThanF cc) yes no) => (FGT cc yes no)
(NZ (GreaterEqualF cc) yes no) => (FGE cc yes no)

(TBNZ [0] (Equal cc) yes no) => (EQ cc yes no)
(TBNZ [0] (NotEqual cc) yes no) => (NE cc yes no)
(TBNZ [0] (LessThan cc) yes no) => (LT cc yes no)
(TBNZ [0] (LessThanU cc) yes no) => (ULT cc yes no)
(TBNZ [0] (LessEqual cc) yes no) => (LE cc yes no)
(TBNZ [0] (LessEqualU cc) yes no) => (ULE cc yes no)
(TBNZ [0] (GreaterThan cc) yes no) => (GT cc yes no)
(TBNZ [0] (GreaterThanU cc) yes no) => (UGT cc yes no)
(TBNZ [0] (GreaterEqual cc) yes no) => (GE cc yes no)
(TBNZ [0] (GreaterEqualU cc) yes no) => (UGE cc yes no)
(TBNZ [0] (LessThanF cc) yes no) => (FLT cc yes no)
(TBNZ [0] (LessEqualF cc) yes no) => (FLE cc yes no)
(TBNZ [0] (GreaterThanF cc) yes no) => (FGT cc yes no)
(TBNZ [0] (GreaterEqualF cc) yes no) => (FGE cc yes no)

(EQ (CMPWconst [0] x:(ANDconst [c] y)) yes no) && x.Uses == 1 => (EQ (TSTWconst [int32(c)] y) yes no)
(NE (CMPWconst [0] x:(ANDconst [c] y)) yes no) && x.Uses == 1 => (NE (TSTWconst [int32(c)] y) yes no)
(LT (CMPWconst [0] x:(ANDconst [c] y)) yes no) && x.Uses == 1 => (LT (TSTWconst [int32(c)] y) yes no)
Expand Down
160 changes: 158 additions & 2 deletions src/cmd/compile/internal/ssa/rewriteARM64.go

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

27 changes: 27 additions & 0 deletions test/fixedbugs/issue52788.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// run

// Copyright 2022 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.

// Issue 52788: miscompilation for boolean comparison on ARM64.

package main

import (
"fmt"
"reflect"
)

func f(next func() bool) {
for b := next(); b; b = next() {
fmt.Printf("next() returned %v\n", b)
}
}

func main() {
next := reflect.MakeFunc(reflect.TypeOf((func() bool)(nil)), func(_ []reflect.Value) []reflect.Value {
return []reflect.Value{reflect.ValueOf(false)}
})
reflect.ValueOf(f).Call([]reflect.Value{next})
}

0 comments on commit e05bd75

Please sign in to comment.