Skip to content

Commit

Permalink
cmd/compile: more fix on boolean ops on ARM64
Browse files Browse the repository at this point in the history
Following CL 405114, 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.

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]>
  • Loading branch information
cherrymui committed May 9, 2022
1 parent f566fe3 commit 90a11e9
Show file tree
Hide file tree
Showing 4 changed files with 177 additions and 5 deletions.
4 changes: 2 additions & 2 deletions src/cmd/compile/internal/ssa/gen/ARM64.rules
Original file line number Diff line number Diff line change
Expand Up @@ -1578,9 +1578,9 @@
(GreaterThanF (InvertFlags x)) => (LessThanF x)
(GreaterEqualF (InvertFlags x)) => (LessEqualF x)

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

// absorb flag constants into conditional instructions
(CSEL [cc] x _ flag) && ccARM64Eval(cc, flag) > 0 => x
Expand Down
148 changes: 145 additions & 3 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.

29 changes: 29 additions & 0 deletions test/fixedbugs/issue52788a.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// 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 ops on ARM64.

package main

import (
"fmt"
"reflect"
"os"
)

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

func main() {
next := reflect.MakeFunc(reflect.TypeOf((func() bool)(nil)), func(_ []reflect.Value) []reflect.Value {
return []reflect.Value{reflect.ValueOf(true)}
})
reflect.ValueOf(f).Call([]reflect.Value{next})
}
1 change: 1 addition & 0 deletions test/fixedbugs/issue52788a.out
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
true

0 comments on commit 90a11e9

Please sign in to comment.