From b136f0c17bdd463207d43e73aef810fa1f14bdee Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Wed, 11 Mar 2020 00:02:04 -0400 Subject: [PATCH] cmd/compile: fix buggy AMD64 rewrite from CL 213058 CL 213058's "bonus optimization I noticed while working on this" turns out to be buggy. It would be correct for CMP, but not TEST. Fix it to use TEST semantics instead. This was breaking compilation with the upcoming Spectre mode. Change-Id: If2d4c3798ed182f35f0244febe74e68c61e4c61b Reviewed-on: https://go-review.googlesource.com/c/go/+/222853 Reviewed-by: Keith Randall --- src/cmd/compile/internal/ssa/gen/AMD64.rules | 5 +- src/cmd/compile/internal/ssa/rewriteAMD64.go | 50 +++++++++++++++++++- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/src/cmd/compile/internal/ssa/gen/AMD64.rules b/src/cmd/compile/internal/ssa/gen/AMD64.rules index 1d24d780c6..07981d2e81 100644 --- a/src/cmd/compile/internal/ssa/gen/AMD64.rules +++ b/src/cmd/compile/internal/ssa/gen/AMD64.rules @@ -1254,7 +1254,10 @@ (CMPWconst (ANDLconst _ [m]) [n]) && 0 <= int16(m) && int16(m) < int16(n) -> (FlagLT_ULT) (CMPBconst (ANDLconst _ [m]) [n]) && 0 <= int8(m) && int8(m) < int8(n) -> (FlagLT_ULT) -(TEST(Q|L)const [c] (MOV(Q|L)const [c])) -> (FlagEQ) +// TESTQ c c sets flags like CMPQ c 0. +(TEST(Q|L)const [c] (MOV(Q|L)const [c])) && c == 0 -> (FlagEQ) +(TEST(Q|L)const [c] (MOV(Q|L)const [c])) && c < 0 -> (FlagLT_UGT) +(TEST(Q|L)const [c] (MOV(Q|L)const [c])) && c > 0 -> (FlagGT_UGT) // TODO: DIVxU also. diff --git a/src/cmd/compile/internal/ssa/rewriteAMD64.go b/src/cmd/compile/internal/ssa/rewriteAMD64.go index bd1f4c08e2..16a3f64158 100644 --- a/src/cmd/compile/internal/ssa/rewriteAMD64.go +++ b/src/cmd/compile/internal/ssa/rewriteAMD64.go @@ -26562,15 +26562,38 @@ func rewriteValueAMD64_OpAMD64TESTL(v *Value) bool { func rewriteValueAMD64_OpAMD64TESTLconst(v *Value) bool { v_0 := v.Args[0] // match: (TESTLconst [c] (MOVLconst [c])) + // cond: c == 0 // result: (FlagEQ) for { c := v.AuxInt - if v_0.Op != OpAMD64MOVLconst || v_0.AuxInt != c { + if v_0.Op != OpAMD64MOVLconst || v_0.AuxInt != c || !(c == 0) { break } v.reset(OpAMD64FlagEQ) return true } + // match: (TESTLconst [c] (MOVLconst [c])) + // cond: c < 0 + // result: (FlagLT_UGT) + for { + c := v.AuxInt + if v_0.Op != OpAMD64MOVLconst || v_0.AuxInt != c || !(c < 0) { + break + } + v.reset(OpAMD64FlagLT_UGT) + return true + } + // match: (TESTLconst [c] (MOVLconst [c])) + // cond: c > 0 + // result: (FlagGT_UGT) + for { + c := v.AuxInt + if v_0.Op != OpAMD64MOVLconst || v_0.AuxInt != c || !(c > 0) { + break + } + v.reset(OpAMD64FlagGT_UGT) + return true + } // match: (TESTLconst [-1] x) // cond: x.Op != OpAMD64MOVLconst // result: (TESTL x x) @@ -26644,15 +26667,38 @@ func rewriteValueAMD64_OpAMD64TESTQ(v *Value) bool { func rewriteValueAMD64_OpAMD64TESTQconst(v *Value) bool { v_0 := v.Args[0] // match: (TESTQconst [c] (MOVQconst [c])) + // cond: c == 0 // result: (FlagEQ) for { c := v.AuxInt - if v_0.Op != OpAMD64MOVQconst || v_0.AuxInt != c { + if v_0.Op != OpAMD64MOVQconst || v_0.AuxInt != c || !(c == 0) { break } v.reset(OpAMD64FlagEQ) return true } + // match: (TESTQconst [c] (MOVQconst [c])) + // cond: c < 0 + // result: (FlagLT_UGT) + for { + c := v.AuxInt + if v_0.Op != OpAMD64MOVQconst || v_0.AuxInt != c || !(c < 0) { + break + } + v.reset(OpAMD64FlagLT_UGT) + return true + } + // match: (TESTQconst [c] (MOVQconst [c])) + // cond: c > 0 + // result: (FlagGT_UGT) + for { + c := v.AuxInt + if v_0.Op != OpAMD64MOVQconst || v_0.AuxInt != c || !(c > 0) { + break + } + v.reset(OpAMD64FlagGT_UGT) + return true + } // match: (TESTQconst [-1] x) // cond: x.Op != OpAMD64MOVQconst // result: (TESTQ x x) -- 2.50.0