[go] cmd/compile: optimize integer comparisons with 1/-1 on riscv64
Michael Munday (Gerrit)
Michael Munday has uploaded this change for review.
cmd/compile: optimize integer comparisons with 1/-1 on riscv64
Some integer comparisons with 1 and -1 can be rewritten as comparisons
with 0. For example, x < 1 is equivalent to x <= 0. This is an
advantageous transformation on riscv64 because comparisons with zero
do not require a constant to be loaded into a register.
The new rules trigger approximately 800 times when building cmd and
std.
Change-Id: I2ce9821dd7605a660eb71d76e83a61f9bae1bf25
---
M src/cmd/compile/internal/ssa/gen/RISCV64.rules
M src/cmd/compile/internal/ssa/rewriteRISCV64.go
M test/codegen/compare_and_branch.go
3 files changed, 116 insertions(+), 0 deletions(-)
diff --git a/src/cmd/compile/internal/ssa/gen/RISCV64.rules b/src/cmd/compile/internal/ssa/gen/RISCV64.rules
index 4290d1b..22fdc64 100644
--- a/src/cmd/compile/internal/ssa/gen/RISCV64.rules
+++ b/src/cmd/compile/internal/ssa/gen/RISCV64.rules
@@ -617,6 +617,18 @@
(BGE (MOVDconst [0]) cond yes no) => (BLEZ cond yes no)
(BGE cond (MOVDconst [0]) yes no) => (BGEZ cond yes no)
+// Convert signed comparison with one to comparison with zero.
+(BGE x (MOVDconst [1]) yes no) => (BGTZ x yes no)
+(BLT x (MOVDconst [1]) yes no) => (BLEZ x yes no)
+
+// Convert signed comparison with minus one to comparison with zero.
+(BGE (MOVDconst [-1]) x yes no) => (BLTZ x yes no)
+(BLT (MOVDconst [-1]) x yes no) => (BGEZ x yes no)
+
+// Convert unsigned comparison with one to comparison with zero.
+(BGEU x (MOVDconst [1]) yes no) => (BGTZ x yes no)
+(BLTU x (MOVDconst [1]) yes no) => (BEQZ x yes no)
+
// Store zero
(MOVBstore [off] {sym} ptr (MOVDconst [0]) mem) => (MOVBstorezero [off] {sym} ptr mem)
(MOVHstore [off] {sym} ptr (MOVDconst [0]) mem) => (MOVHstorezero [off] {sym} ptr mem)
diff --git a/src/cmd/compile/internal/ssa/rewriteRISCV64.go b/src/cmd/compile/internal/ssa/rewriteRISCV64.go
index f856a26..c283345 100644
--- a/src/cmd/compile/internal/ssa/rewriteRISCV64.go
+++ b/src/cmd/compile/internal/ssa/rewriteRISCV64.go
@@ -6364,6 +6364,40 @@
b.resetWithControl(BlockRISCV64BGEZ, cond)
return true
}
+ // match: (BGE x (MOVDconst [1]) yes no)
+ // result: (BGTZ x yes no)
+ for b.Controls[1].Op == OpRISCV64MOVDconst {
+ x := b.Controls[0]
+ v_1 := b.Controls[1]
+ if auxIntToInt64(v_1.AuxInt) != 1 {
+ break
+ }
+ b.resetWithControl(BlockRISCV64BGTZ, x)
+ return true
+ }
+ // match: (BGE (MOVDconst [-1]) x yes no)
+ // result: (BLTZ x yes no)
+ for b.Controls[0].Op == OpRISCV64MOVDconst {
+ v_0 := b.Controls[0]
+ if auxIntToInt64(v_0.AuxInt) != -1 {
+ break
+ }
+ x := b.Controls[1]
+ b.resetWithControl(BlockRISCV64BLTZ, x)
+ return true
+ }
+ case BlockRISCV64BGEU:
+ // match: (BGEU x (MOVDconst [1]) yes no)
+ // result: (BGTZ x yes no)
+ for b.Controls[1].Op == OpRISCV64MOVDconst {
+ x := b.Controls[0]
+ v_1 := b.Controls[1]
+ if auxIntToInt64(v_1.AuxInt) != 1 {
+ break
+ }
+ b.resetWithControl(BlockRISCV64BGTZ, x)
+ return true
+ }
case BlockRISCV64BLT:
// match: (BLT (MOVDconst [0]) cond yes no)
// result: (BGTZ cond yes no)
@@ -6387,6 +6421,40 @@
b.resetWithControl(BlockRISCV64BLTZ, cond)
return true
}
+ // match: (BLT x (MOVDconst [1]) yes no)
+ // result: (BLEZ x yes no)
+ for b.Controls[1].Op == OpRISCV64MOVDconst {
+ x := b.Controls[0]
+ v_1 := b.Controls[1]
+ if auxIntToInt64(v_1.AuxInt) != 1 {
+ break
+ }
+ b.resetWithControl(BlockRISCV64BLEZ, x)
+ return true
+ }
+ // match: (BLT (MOVDconst [-1]) x yes no)
+ // result: (BGEZ x yes no)
+ for b.Controls[0].Op == OpRISCV64MOVDconst {
+ v_0 := b.Controls[0]
+ if auxIntToInt64(v_0.AuxInt) != -1 {
+ break
+ }
+ x := b.Controls[1]
+ b.resetWithControl(BlockRISCV64BGEZ, x)
+ return true
+ }
+ case BlockRISCV64BLTU:
+ // match: (BLTU x (MOVDconst [1]) yes no)
+ // result: (BEQZ x yes no)
+ for b.Controls[1].Op == OpRISCV64MOVDconst {
+ x := b.Controls[0]
+ v_1 := b.Controls[1]
+ if auxIntToInt64(v_1.AuxInt) != 1 {
+ break
+ }
+ b.resetWithControl(BlockRISCV64BEQZ, x)
+ return true
+ }
case BlockRISCV64BNE:
// match: (BNE (MOVDconst [0]) cond yes no)
// result: (BNEZ cond yes no)
diff --git a/test/codegen/compare_and_branch.go b/test/codegen/compare_and_branch.go
index f751506..d45902f 100644
--- a/test/codegen/compare_and_branch.go
+++ b/test/codegen/compare_and_branch.go
@@ -204,3 +204,39 @@
dummy()
}
}
+
+// Signed 64-bit comparison with 1/-1 to comparison with 0.
+func si64x0(x chan int64) {
+ // riscv64:"BGTZ"
+ for <-x >= 1 {
+ dummy()
+ }
+
+ // riscv64:"BLEZ"
+ for <-x < 1 {
+ dummy()
+ }
+
+ // riscv64:"BLTZ"
+ for <-x <= -1 {
+ dummy()
+ }
+
+ // riscv64:"BGEZ"
+ for <-x > -1 {
+ dummy()
+ }
+}
+
+// Unsigned 64-bit comparison with 1 to comparison with 0.
+func ui64x0(x chan uint64) {
+ // riscv64:"BGTZ"
+ for <-x >= 1 {
+ dummy()
+ }
+
+ // riscv64:"BEQZ"
+ for <-x < 1 {
+ dummy()
+ }
+}
To view, visit change 350831. To unsubscribe, or for help writing mail filters, visit settings.
Michael Munday (Gerrit)
Michael Munday removed Keith Randall from this change.
To view, visit change 350831. To unsubscribe, or for help writing mail filters, visit settings.
Michael Munday (Gerrit)
Michael Munday removed Josh Bleecher Snyder from this change.
To view, visit change 350831. To unsubscribe, or for help writing mail filters, visit settings.
Michael Munday (Gerrit)
Michael Munday removed Martin Möhrmann from this change.
To view, visit change 350831. To unsubscribe, or for help writing mail filters, visit settings.
Michael Munday (Gerrit)
Michael Munday removed Go Bot from this change.
To view, visit change 350831. To unsubscribe, or for help writing mail filters, visit settings.
Michael Munday (Gerrit)
Michael Munday removed Keith Randall from this change.
To view, visit change 350831. To unsubscribe, or for help writing mail filters, visit settings.
Michael Munday (Gerrit)
Michael Munday removed Josh Bleecher Snyder from this change.
To view, visit change 350831. To unsubscribe, or for help writing mail filters, visit settings.
Michael Munday (Gerrit)
Michael Munday removed Martin Möhrmann from this change.
To view, visit change 350831. To unsubscribe, or for help writing mail filters, visit settings.
Michael Munday (Gerrit)
Patch set 1:Run-TryBot +1Trust +1
1 comment:
Patchset:
TRY=riscv64
To view, visit change 350831. To unsubscribe, or for help writing mail filters, visit settings.
Keith Randall (Gerrit)
Attention is currently required from: Michael Munday.
1 comment:
Patchset:
There was another similar CL that went in recently, CL 346050. Is this related to / redundant to / ... that one?
It works on a generic level, which might also be worth doing here, as maybe many architectures would benefit (and none would be made worse, I think).
To view, visit change 350831. To unsubscribe, or for help writing mail filters, visit settings.
Michael Munday (Gerrit)
Attention is currently required from: Keith Randall.
2 comments:
Patchset:
There was another similar CL that went in recently, CL 346050. Is this related to / redundant to / . […]
Interesting, I wasn't aware of that. Yeah, a generic approach would probably be better. This CL covers more cases than CL 346050. I'll do some experimentation. Thanks Keith!
(There is a bug here: the BGEU rule needs to use an unsigned version of BGTZ, unfortunately BGTUZ doesn't currently exist as a pseudo instruction.)
To view, visit change 350831. To unsubscribe, or for help writing mail filters, visit settings.
Michael Munday (Gerrit)
Attention is currently required from: Keith Randall.
Michael Munday uploaded patch set #2 to this change.
cmd/compile: use zero constants in comparisons where possible
Some integer comparisons with 1 and -1 can be rewritten as comparisons
with 0. For example, x < 1 is equivalent to x <= 0. This is an
advantageous transformation on riscv64 because comparisons with zero
do not require a constant to be loaded into a register. Other
architectures will likely benefit too and the transformation is
relatively benign on architectures that do not benefit.
Change-Id: I2ce9821dd7605a660eb71d76e83a61f9bae1bf25
---
M src/cmd/compile/internal/ssa/gen/generic.rules
M src/cmd/compile/internal/ssa/rewritegeneric.go
M test/codegen/compare_and_branch.go
3 files changed, 465 insertions(+), 0 deletions(-)
To view, visit change 350831. To unsubscribe, or for help writing mail filters, visit settings.
Michael Munday (Gerrit)
Attention is currently required from: Keith Randall.
Patch set 2:Run-TryBot +1Trust +1
To view, visit change 350831. To unsubscribe, or for help writing mail filters, visit settings.
Michael Munday (Gerrit)
Attention is currently required from: Michael Munday, Keith Randall.
Michael Munday uploaded patch set #3 to this change.
cmd/compile: use zero constants in comparisons where possible
Some integer comparisons with 1 and -1 can be rewritten as comparisons
with 0. For example, x < 1 is equivalent to x <= 0. This is an
advantageous transformation on riscv64 because comparisons with zero
do not require a constant to be loaded into a register. Other
architectures will likely benefit too and the transformation is
relatively benign on architectures that do not benefit.
Change-Id: I2ce9821dd7605a660eb71d76e83a61f9bae1bf25
---
M src/cmd/compile/internal/ssa/gen/generic.rules
M src/cmd/compile/internal/ssa/rewritegeneric.go
M test/codegen/compare_and_branch.go
M test/prove.go
4 files changed, 466 insertions(+), 1 deletion(-)
To view, visit change 350831. To unsubscribe, or for help writing mail filters, visit settings.
Michael Munday (Gerrit)
Attention is currently required from: Keith Randall.
Patch set 3:Run-TryBot +1Trust +1
1 comment:
Patchset:
TRY=
To view, visit change 350831. To unsubscribe, or for help writing mail filters, visit settings.
Michael Munday (Gerrit)
Attention is currently required from: Keith Randall.
2 comments:
Patchset:
(There is a bug here: the BGEU rule needs to use an unsigned version of BGTZ, unfortunately BGTUZ do […]
(This is what I get for trying to stay up late doing this sort of stuff - BGTUZ is of course BNEZ).
Patchset:
I think the new rules are reasonable but I'm not sure they carry their weight. compilecmp results just looked like noise for riscv64. That said they do improve the code generated in the examples given.
To view, visit change 350831. To unsubscribe, or for help writing mail filters, visit settings.
Michael Munday (Gerrit)
Attention is currently required from: Keith Randall.
1 comment:
Patchset:
I think the new rules are reasonable but I'm not sure they carry their weight. […]
Note to self: double check that this doesn't interfere with range-check detection. Comparisons with 1 seem to often be used to test if the value is within a certain range.
To view, visit change 350831. To unsubscribe, or for help writing mail filters, visit settings.
Keith Randall (Gerrit)
Attention is currently required from: Michael Munday.
Patch set 3:Code-Review +2
1 comment:
Patchset:
I'm ok with changes that make the generated code better, even if compilecmp doesn't show a difference. As long as it doesn't show a slowdown.
To view, visit change 350831. To unsubscribe, or for help writing mail filters, visit settings.
Michael Knyszek (Gerrit)
Attention is currently required from: Michael Munday.
Patch set 3:Code-Review +1
Ian Lance Taylor (Gerrit)
Attention is currently required from: Michael Munday.
Patch set 3:Code-Review +1
To view, visit change 350831. To unsubscribe, or for help writing mail filters, visit settings.
Daniel Martí (Gerrit)
Attention is currently required from: Michael Munday.
1 comment:
Patchset:
Should we rebase and re-test this to merge it?
To view, visit change 350831. To unsubscribe, or for help writing mail filters, visit settings.
Michael Munday (Gerrit)
Attention is currently required from: Michael Munday.
Michael Munday uploaded patch set #4 to this change.
The following approvals got outdated and were removed: Run-TryBot+1 by Michael Munday, Trust+1 by Michael Munday, TryBot-Result+1 by Gopher Robot
The change is no longer submittable: TryBots-Pass is unsatisfied now.
cmd/compile: use zero constants in comparisons where possible
Some integer comparisons with 1 and -1 can be rewritten as comparisons
with 0. For example, x < 1 is equivalent to x <= 0. This is an
advantageous transformation on riscv64 because comparisons with zero
do not require a constant to be loaded into a register. Other
architectures will likely benefit too and the transformation is
relatively benign on architectures that do not benefit.
Change-Id: I2ce9821dd7605a660eb71d76e83a61f9bae1bf25
---
M src/cmd/compile/internal/ssa/_gen/generic.rules
M src/cmd/compile/internal/ssa/rewritegeneric.go
M test/codegen/compare_and_branch.go
M test/prove.go
4 files changed, 482 insertions(+), 1 deletion(-)
To view, visit change 350831. To unsubscribe, or for help writing mail filters, visit settings.
Michael Munday (Gerrit)
Patch set 4:Run-TryBot +1