Skip to content

Commit e1099eb

Browse files
cherrymuithanm
authored andcommitted
[release-branch.go1.18] cmd/compile: more fix on boolean ops on ARM64
Following CL 421457, the extension rule is also wrong. It is safe to drop the extension if the value is from a boolean-generating instruction, but not a boolean-typed Value in general (e.g. a Phi or a in-register parameter). Fix it. Updates #52788. Updates #53397. Change-Id: Icf3028fe8e90806f9f57fbe2b38d47da27a97e2a Reviewed-on: https://go-review.googlesource.com/c/go/+/405115 Reviewed-by: David Chase <[email protected]> Run-TryBot: Cherry Mui <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/go/+/421458 Reviewed-by: Cherry Mui <[email protected]> Run-TryBot: Dmitri Shuralyov <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]>
1 parent e05bd75 commit e1099eb

File tree

4 files changed

+177
-5
lines changed

4 files changed

+177
-5
lines changed

src/cmd/compile/internal/ssa/gen/ARM64.rules

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1578,9 +1578,9 @@
15781578
(GreaterThanF (InvertFlags x)) => (LessThanF x)
15791579
(GreaterEqualF (InvertFlags x)) => (LessEqualF x)
15801580

1581-
// Boolean-generating instructions always
1581+
// Boolean-generating instructions (NOTE: NOT all boolean Values) always
15821582
// zero upper bit of the register; no need to zero-extend
1583-
(MOVBUreg x) && x.Type.IsBoolean() => (MOVDreg x)
1583+
(MOVBUreg x:((Equal|NotEqual|LessThan|LessThanU|LessThanF|LessEqual|LessEqualU|LessEqualF|GreaterThan|GreaterThanU|GreaterThanF|GreaterEqual|GreaterEqualU|GreaterEqualF) _)) => (MOVDreg x)
15841584

15851585
// absorb flag constants into conditional instructions
15861586
(CSEL [cc] x _ flag) && ccARM64Eval(cc, flag) > 0 => x

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

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

test/fixedbugs/issue52788a.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// run
2+
3+
// Copyright 2022 The Go Authors. All rights reserved.
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file.
6+
7+
// Issue 52788: miscompilation for boolean ops on ARM64.
8+
9+
package main
10+
11+
import (
12+
"fmt"
13+
"reflect"
14+
"os"
15+
)
16+
17+
func f(next func() bool) {
18+
for b := next(); b; b = next() {
19+
fmt.Printf("%v\n", b)
20+
os.Exit(0)
21+
}
22+
}
23+
24+
func main() {
25+
next := reflect.MakeFunc(reflect.TypeOf((func() bool)(nil)), func(_ []reflect.Value) []reflect.Value {
26+
return []reflect.Value{reflect.ValueOf(true)}
27+
})
28+
reflect.ValueOf(f).Call([]reflect.Value{next})
29+
}

test/fixedbugs/issue52788a.out

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
true

0 commit comments

Comments
 (0)