From 8d77e45064fc808460843e741c46e3b6b737ae03 Mon Sep 17 00:00:00 2001 From: eric fang Date: Tue, 6 Apr 2021 07:23:38 +0000 Subject: [PATCH] cmd/compile: fix bug of conditional instructions on arm64 CL 302231 added some optimization rules with instructions CSETM, CSINC, CSINV, and CSNEG, but did not deal with the situation where flag is constant, resulting in some cases that could be more optimized cannot be optimized, and the FlagConstant value is passed to codegen pass. This CL adds these missing rules. Fixes #45359 Change-Id: I700608cfb9a6a768a18d1fd5d374d7e92aa6f838 Reviewed-on: https://go-review.googlesource.com/c/go/+/307650 Reviewed-by: eric fang Reviewed-by: Keith Randall Reviewed-by: Cherry Zhang Reviewed-by: Alberto Donizetti Run-TryBot: eric fang TryBot-Result: Go Bot Trust: eric fang Trust: Alberto Donizetti --- src/cmd/compile/internal/ssa/gen/ARM64.rules | 8 ++ src/cmd/compile/internal/ssa/rewriteARM64.go | 108 +++++++++++++++++++ test/fixedbugs/issue45359.go | 20 ++++ 3 files changed, 136 insertions(+) create mode 100644 test/fixedbugs/issue45359.go diff --git a/src/cmd/compile/internal/ssa/gen/ARM64.rules b/src/cmd/compile/internal/ssa/gen/ARM64.rules index cb997776db..3d2759493e 100644 --- a/src/cmd/compile/internal/ssa/gen/ARM64.rules +++ b/src/cmd/compile/internal/ssa/gen/ARM64.rules @@ -1570,6 +1570,14 @@ (CSEL [cc] _ y flag) && ccARM64Eval(cc, flag) < 0 => y (CSEL0 [cc] x flag) && ccARM64Eval(cc, flag) > 0 => x (CSEL0 [cc] _ flag) && ccARM64Eval(cc, flag) < 0 => (MOVDconst [0]) +(CSNEG [cc] x _ flag) && ccARM64Eval(cc, flag) > 0 => x +(CSNEG [cc] _ y flag) && ccARM64Eval(cc, flag) < 0 => (NEG y) +(CSINV [cc] x _ flag) && ccARM64Eval(cc, flag) > 0 => x +(CSINV [cc] _ y flag) && ccARM64Eval(cc, flag) < 0 => (Not y) +(CSINC [cc] x _ flag) && ccARM64Eval(cc, flag) > 0 => x +(CSINC [cc] _ y flag) && ccARM64Eval(cc, flag) < 0 => (ADDconst [1] y) +(CSETM [cc] flag) && ccARM64Eval(cc, flag) > 0 => (MOVDconst [-1]) +(CSETM [cc] flag) && ccARM64Eval(cc, flag) < 0 => (MOVDconst [0]) // absorb flags back into boolean CSEL (CSEL [cc] x y (CMPWconst [0] boolval)) && cc == OpARM64NotEqual && flagArg(boolval) != nil => diff --git a/src/cmd/compile/internal/ssa/rewriteARM64.go b/src/cmd/compile/internal/ssa/rewriteARM64.go index cdc246205c..0ba3951df5 100644 --- a/src/cmd/compile/internal/ssa/rewriteARM64.go +++ b/src/cmd/compile/internal/ssa/rewriteARM64.go @@ -3619,6 +3619,32 @@ func rewriteValueARM64_OpARM64CSETM(v *Value) bool { v.AddArg(cmp) return true } + // match: (CSETM [cc] flag) + // cond: ccARM64Eval(cc, flag) > 0 + // result: (MOVDconst [-1]) + for { + cc := auxIntToOp(v.AuxInt) + flag := v_0 + if !(ccARM64Eval(cc, flag) > 0) { + break + } + v.reset(OpARM64MOVDconst) + v.AuxInt = int64ToAuxInt(-1) + return true + } + // match: (CSETM [cc] flag) + // cond: ccARM64Eval(cc, flag) < 0 + // result: (MOVDconst [0]) + for { + cc := auxIntToOp(v.AuxInt) + flag := v_0 + if !(ccARM64Eval(cc, flag) < 0) { + break + } + v.reset(OpARM64MOVDconst) + v.AuxInt = int64ToAuxInt(0) + return true + } return false } func rewriteValueARM64_OpARM64CSINC(v *Value) bool { @@ -3640,6 +3666,34 @@ func rewriteValueARM64_OpARM64CSINC(v *Value) bool { v.AddArg3(x, y, cmp) return true } + // match: (CSINC [cc] x _ flag) + // cond: ccARM64Eval(cc, flag) > 0 + // result: x + for { + cc := auxIntToOp(v.AuxInt) + x := v_0 + flag := v_2 + if !(ccARM64Eval(cc, flag) > 0) { + break + } + v.copyOf(x) + return true + } + // match: (CSINC [cc] _ y flag) + // cond: ccARM64Eval(cc, flag) < 0 + // result: (ADDconst [1] y) + for { + cc := auxIntToOp(v.AuxInt) + y := v_1 + flag := v_2 + if !(ccARM64Eval(cc, flag) < 0) { + break + } + v.reset(OpARM64ADDconst) + v.AuxInt = int64ToAuxInt(1) + v.AddArg(y) + return true + } return false } func rewriteValueARM64_OpARM64CSINV(v *Value) bool { @@ -3661,6 +3715,33 @@ func rewriteValueARM64_OpARM64CSINV(v *Value) bool { v.AddArg3(x, y, cmp) return true } + // match: (CSINV [cc] x _ flag) + // cond: ccARM64Eval(cc, flag) > 0 + // result: x + for { + cc := auxIntToOp(v.AuxInt) + x := v_0 + flag := v_2 + if !(ccARM64Eval(cc, flag) > 0) { + break + } + v.copyOf(x) + return true + } + // match: (CSINV [cc] _ y flag) + // cond: ccARM64Eval(cc, flag) < 0 + // result: (Not y) + for { + cc := auxIntToOp(v.AuxInt) + y := v_1 + flag := v_2 + if !(ccARM64Eval(cc, flag) < 0) { + break + } + v.reset(OpNot) + v.AddArg(y) + return true + } return false } func rewriteValueARM64_OpARM64CSNEG(v *Value) bool { @@ -3682,6 +3763,33 @@ func rewriteValueARM64_OpARM64CSNEG(v *Value) bool { v.AddArg3(x, y, cmp) return true } + // match: (CSNEG [cc] x _ flag) + // cond: ccARM64Eval(cc, flag) > 0 + // result: x + for { + cc := auxIntToOp(v.AuxInt) + x := v_0 + flag := v_2 + if !(ccARM64Eval(cc, flag) > 0) { + break + } + v.copyOf(x) + return true + } + // match: (CSNEG [cc] _ y flag) + // cond: ccARM64Eval(cc, flag) < 0 + // result: (NEG y) + for { + cc := auxIntToOp(v.AuxInt) + y := v_1 + flag := v_2 + if !(ccARM64Eval(cc, flag) < 0) { + break + } + v.reset(OpARM64NEG) + v.AddArg(y) + return true + } return false } func rewriteValueARM64_OpARM64DIV(v *Value) bool { diff --git a/test/fixedbugs/issue45359.go b/test/fixedbugs/issue45359.go new file mode 100644 index 0000000000..5448556768 --- /dev/null +++ b/test/fixedbugs/issue45359.go @@ -0,0 +1,20 @@ +// compile + +// Copyright 2021 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. + +package main + +func f() { + var i, j int + var b bool + i = -(i &^ i) + for 1>>uint(i) == 0 { + _ = func() { + i, b = 0, true + } + _ = b + i %= j + } +}