Skip to content

Commit

Permalink
[release-branch.go1.18] cmd/compile: more fix on boolean ops on ARM64
Browse files Browse the repository at this point in the history
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 <drchase@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/421458
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
  • Loading branch information
cherrymui authored and thanm committed Aug 8, 2022
1 parent e05bd75 commit e1099eb
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 e1099eb

Please sign in to comment.