Skip to content

Commit fcd2807

Browse files
committed
cmd/compile: add opt branchelim to rewrite some CondSelect into math
This allows something like: if y { x++ } To be compiled to: MOVBLZX BX, CX ADDQ CX, AX Instead of: LEAQ 1(AX), CX MOVBLZX BL, DX TESTQ DX, DX CMOVQNE CX, AX While ./make.bash uniqued per LOC, there is 100 additions and 75 substractions. See benchmark here: https://go.dev/play/p/DJf5COjwhd_s Either it's a performance no-op or it is faster: goos: linux goarch: amd64 cpu: AMD Ryzen 5 3600 6-Core Processor │ /tmp/old.logs │ /tmp/new.logs │ │ sec/op │ sec/op vs base │ CmovInlineConditionAddLatency-12 0.5443n ± 5% 0.5339n ± 3% -1.90% (p=0.004 n=10) CmovInlineConditionAddThroughputBy6-12 1.492n ± 1% 1.494n ± 1% ~ (p=0.955 n=10) CmovInlineConditionSubLatency-12 0.5419n ± 3% 0.5282n ± 3% -2.52% (p=0.019 n=10) CmovInlineConditionSubThroughputBy6-12 1.587n ± 1% 1.584n ± 2% ~ (p=0.492 n=10) CmovOutlineConditionAddLatency-12 0.5223n ± 1% 0.2639n ± 4% -49.47% (p=0.000 n=10) CmovOutlineConditionAddThroughputBy6-12 1.159n ± 1% 1.097n ± 2% -5.35% (p=0.000 n=10) CmovOutlineConditionSubLatency-12 0.5271n ± 3% 0.2654n ± 2% -49.66% (p=0.000 n=10) CmovOutlineConditionSubThroughputBy6-12 1.053n ± 1% 1.050n ± 1% ~ (p=1.000 n=10) geomean There are other benefits not tested by this benchmark: - the math form is usually a couple bytes shorter (ICACHE) - the math form is usually 0~2 uops shorter (UCACHE) - the math form has usually less register pressure* - the math form can sometimes be optimized further *regalloc rarely find how it can use less registers As far as pass ordering goes there are many possible options, I've decided to reorder branchelim before late opt since: - unlike running exclusively the CondSelect rules after branchelim, some extra optimizations might trigger on the adds or subs. - I don't want to maintain a second generic.rules file of only the stuff, that can trigger after branchelim. - rerunning all of opt a third time increase compilation time for little gains. By elimination moving branchelim seems fine. Change-Id: I869adf57e4d109948ee157cfc47144445146bafd Reviewed-on: https://go-review.googlesource.com/c/go/+/685676 Reviewed-by: Keith Randall <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Keith Randall <[email protected]> Reviewed-by: Michael Knyszek <[email protected]>
1 parent f32cf8e commit fcd2807

File tree

4 files changed

+292
-4
lines changed

4 files changed

+292
-4
lines changed

src/cmd/compile/internal/ssa/_gen/generic.rules

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,9 @@
295295
(Neq16 (Const16 <t> [c]) (Add16 (Const16 <t> [d]) x)) => (Neq16 (Const16 <t> [c-d]) x)
296296
(Neq8 (Const8 <t> [c]) (Add8 (Const8 <t> [d]) x)) => (Neq8 (Const8 <t> [c-d]) x)
297297

298+
(CondSelect x _ (ConstBool [true ])) => x
299+
(CondSelect _ y (ConstBool [false])) => y
300+
298301
// signed integer range: ( c <= x && x (<|<=) d ) -> ( unsigned(x-c) (<|<=) unsigned(d-c) )
299302
(AndB (Leq64 (Const64 [c]) x) ((Less|Leq)64 x (Const64 [d]))) && d >= c => ((Less|Leq)64U (Sub64 <x.Type> x (Const64 <x.Type> [c])) (Const64 <x.Type> [d-c]))
300303
(AndB (Leq32 (Const32 [c]) x) ((Less|Leq)32 x (Const32 [d]))) && d >= c => ((Less|Leq)32U (Sub32 <x.Type> x (Const32 <x.Type> [c])) (Const32 <x.Type> [d-c]))
@@ -2843,3 +2846,12 @@
28432846
&& clobber(sbts)
28442847
&& clobber(key)
28452848
=> (StaticLECall {f} [argsize] dict_ (StringMake <typ.String> ptr len) mem)
2849+
2850+
// Transform some CondSelect into math operations.
2851+
// if b { x++ } => x += b // but not on arm64 because it has CSINC
2852+
(CondSelect (Add8 <t> x (Const8 [1])) x bool) && config.arch != "arm64" => (Add8 x (CvtBoolToUint8 <t> bool))
2853+
(CondSelect (Add(64|32|16) <t> x (Const(64|32|16) [1])) x bool) && config.arch != "arm64" => (Add(64|32|16) x (ZeroExt8to(64|32|16) <t> (CvtBoolToUint8 <types.Types[types.TUINT8]> bool)))
2854+
2855+
// if b { x-- } => x -= b
2856+
(CondSelect (Add8 <t> x (Const8 [-1])) x bool) => (Sub8 x (CvtBoolToUint8 <t> bool))
2857+
(CondSelect (Add(64|32|16) <t> x (Const(64|32|16) [-1])) x bool) => (Sub(64|32|16) x (ZeroExt8to(64|32|16) <t> (CvtBoolToUint8 <types.Types[types.TUINT8]> bool)))

src/cmd/compile/internal/ssa/compile.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -473,11 +473,11 @@ var passes = [...]pass{
473473
{name: "expand calls", fn: expandCalls, required: true},
474474
{name: "decompose builtin", fn: postExpandCallsDecompose, required: true},
475475
{name: "softfloat", fn: softfloat, required: true},
476+
{name: "branchelim", fn: branchelim},
476477
{name: "late opt", fn: opt, required: true}, // TODO: split required rules and optimizing rules
477478
{name: "dead auto elim", fn: elimDeadAutosGeneric},
478479
{name: "sccp", fn: sccp},
479480
{name: "generic deadcode", fn: deadcode, required: true}, // remove dead stores, which otherwise mess up store chain
480-
{name: "branchelim", fn: branchelim},
481481
{name: "late fuse", fn: fuseLate},
482482
{name: "check bce", fn: checkbce},
483483
{name: "dse", fn: dse},
@@ -583,6 +583,10 @@ var passOrder = [...]constraint{
583583
{"late fuse", "memcombine"},
584584
// memcombine is a arch-independent pass.
585585
{"memcombine", "lower"},
586+
// late opt transform some CondSelects into math.
587+
{"branchelim", "late opt"},
588+
// ranchelim is an arch-independent pass.
589+
{"branchelim", "lower"},
586590
}
587591

588592
func init() {

src/cmd/compile/internal/ssa/rewritegeneric.go

Lines changed: 250 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/codegen/condmove.go

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ func cmovfloatint2(x, y float64) float64 {
106106
for r >= y {
107107
rfr, rexp := frexp(r)
108108
if rfr < yfr {
109-
rexp = rexp - 1
109+
rexp = rexp - 42
110110
}
111111
// amd64:"CMOVQHI"
112112
// arm64:"CSEL\tMI"
@@ -205,7 +205,7 @@ func cmovinvert6(x, y uint64) uint64 {
205205

206206
func cmovload(a []int, i int, b bool) int {
207207
if b {
208-
i++
208+
i += 42
209209
}
210210
// See issue 26306
211211
// amd64:-"CMOVQNE"
@@ -214,7 +214,7 @@ func cmovload(a []int, i int, b bool) int {
214214

215215
func cmovstore(a []int, i int, b bool) {
216216
if b {
217-
i++
217+
i += 42
218218
}
219219
// amd64:"CMOVQNE"
220220
a[i] = 7
@@ -451,3 +451,25 @@ func cmovzeroreg1(a, b int) int {
451451
// ppc64x:"ISEL\t[$]2, R0, R[0-9]+, R[0-9]+"
452452
return x
453453
}
454+
455+
func cmovmathadd(a uint, b bool) uint {
456+
if b {
457+
a++
458+
}
459+
// amd64:"ADDQ", -"CMOV"
460+
// arm64:"CSINC", -"CSEL"
461+
// ppc64x:"ADD", -"ISEL"
462+
// wasm:"Add", "-Select"
463+
return a
464+
}
465+
466+
func cmovmathsub(a uint, b bool) uint {
467+
if b {
468+
a--
469+
}
470+
// amd64:"SUBQ", -"CMOV"
471+
// arm64:"SUB", -"CSEL"
472+
// ppc64x:"SUB", -"ISEL"
473+
// wasm:"Sub", "-Select"
474+
return a
475+
}

0 commit comments

Comments
 (0)