From 822a9f537fb49f56d405f265fa4d1d3e9ddc0531 Mon Sep 17 00:00:00 2001 From: fanzha02 Date: Sun, 5 May 2019 03:35:37 +0000 Subject: [PATCH] cmd/compile: fix the error of absorbing boolean tests into block(FGE, FGT) The CL 164718 mistyped the comparison flags. The rules for floating point comparison should be GreaterThanF and GreaterEqualF. Fortunately, the wrong optimizations were overwritten by other integer rules, so the issue won't cause failure but just some performance impact. The fixed CL optimizes the floating point test as follows. source code: func foo(f float64) bool { return f > 4 || f < -4} previous version: "FCMPD", "CSET\tGT", "CBZ" fixed version: "FCMPD", BLE" Add the test case. Change-Id: Iea954fdbb8272b2d642dae0f816dc77286e6e1fa Reviewed-on: https://go-review.googlesource.com/c/go/+/177121 Reviewed-by: Ben Shi Reviewed-by: Cherry Zhang Run-TryBot: Ben Shi TryBot-Result: Gobot Gobot --- src/cmd/compile/internal/ssa/gen/ARM64.rules | 4 ++-- src/cmd/compile/internal/ssa/rewriteARM64.go | 8 ++++---- test/codegen/floats.go | 5 +++++ 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/cmd/compile/internal/ssa/gen/ARM64.rules b/src/cmd/compile/internal/ssa/gen/ARM64.rules index f3f006905c7..d4b47bfb0b7 100644 --- a/src/cmd/compile/internal/ssa/gen/ARM64.rules +++ b/src/cmd/compile/internal/ssa/gen/ARM64.rules @@ -630,8 +630,8 @@ (NZ (GreaterEqualU cc) yes no) -> (UGE cc yes no) (NZ (LessThanF cc) yes no) -> (FLT cc yes no) (NZ (LessEqualF cc) yes no) -> (FLE cc yes no) -(NZ (GreaterThan cc) yes no) -> (FGT cc yes no) -(NZ (GreaterEqual cc) yes no) -> (FGE cc yes no) +(NZ (GreaterThanF cc) yes no) -> (FGT cc yes no) +(NZ (GreaterEqualF cc) yes no) -> (FGE cc yes no) (EQ (CMPWconst [0] x:(ANDconst [c] y)) yes no) && x.Uses == 1 -> (EQ (TSTWconst [c] y) yes no) (NE (CMPWconst [0] x:(ANDconst [c] y)) yes no) && x.Uses == 1 -> (NE (TSTWconst [c] y) yes no) diff --git a/src/cmd/compile/internal/ssa/rewriteARM64.go b/src/cmd/compile/internal/ssa/rewriteARM64.go index 7c3f3b9e0ca..9dfd848bc4e 100644 --- a/src/cmd/compile/internal/ssa/rewriteARM64.go +++ b/src/cmd/compile/internal/ssa/rewriteARM64.go @@ -41214,20 +41214,20 @@ func rewriteBlockARM64(b *Block) bool { b.Aux = nil return true } - // match: (NZ (GreaterThan cc) yes no) + // match: (NZ (GreaterThanF cc) yes no) // cond: // result: (FGT cc yes no) - for v.Op == OpARM64GreaterThan { + for v.Op == OpARM64GreaterThanF { cc := v.Args[0] b.Kind = BlockARM64FGT b.SetControl(cc) b.Aux = nil return true } - // match: (NZ (GreaterEqual cc) yes no) + // match: (NZ (GreaterEqualF cc) yes no) // cond: // result: (FGE cc yes no) - for v.Op == OpARM64GreaterEqual { + for v.Op == OpARM64GreaterEqualF { cc := v.Args[0] b.Kind = BlockARM64FGE b.SetControl(cc) diff --git a/test/codegen/floats.go b/test/codegen/floats.go index 5e1f60b08ba..7ec36549814 100644 --- a/test/codegen/floats.go +++ b/test/codegen/floats.go @@ -117,6 +117,11 @@ func FusedSub64_b(x, y, z float64) float64 { return z - x*y } +func Cmp(f float64) bool { + // arm64:"FCMPD","BLE",-"CSET\tGT",-"CBZ" + return f > 4 || f < -4 +} + // ---------------- // // Non-floats // // ---------------- //