Branch comparison folding and order optimizations #629

tyler · 2018-12-21T00:56:33Z

Fixes #606.

This PR add two optimizations to simple_preopt.rs. The first is to fold code where we compare against zero, then use an explicit conditional branch. For instance:

v1 = icmp_imm v0, 0
brz v1, ebb1

becomes

brnz v0, ebb1

For x86-64 this doesn't actually make much difference, as it compiles to a test rather than a cmp, which should be very near equivalent. But it does save a byte on a very common operation, so hey.

The second is reordering branches at the end of ebbs to encourage fallthroughs. As pointed out in #606, it's common for wasm code to be compiled like this:

    brz v0, ebb1
    jump ebb2
ebb1:
    ...
ebb2:
    ...

Which ends up being compiled as a conditional jump followed by a jump. However, we can switch those instructions by flipping the destinations and inverting the conditional jump's condition, like so:

    brnz v0, ebb2
    fallthrough ebb1
ebb1:
    ...
ebb2:
    ...

And when we do that we eliminate the latter jump entirely.

lib/codegen/src/simple_preopt.rs

sunfishcode

Looks like this is on the right track!

Long term, I think this is additional motivation for Cranelift developing more convenient APIs for pattern-matching, as it's a fair amount of work to dig out all the required bits and test all the needed conditions manually like this :-).

lib/codegen/src/simple_preopt.rs

tyler · 2019-01-03T22:06:09Z

Cargo.toml

@@ -14,7 +14,7 @@ name = "clif-util"
 path = "src/clif-util.rs"

 [dependencies]
-cfg-if = "0.1"
+cfg-if = "0.1.6"


This appears to have been underspecified. With v0.1.4 locally, I was getting errors about missing macros. Presumably due to lack of this commit: rust-lang/cfg-if@67e01e5

tyler · 2019-01-04T20:45:32Z

I think this is ready for another round of reviews.

sunfishcode

Cool, overall this looks good!

lib/codegen/src/simple_preopt.rs

bnjbvr · 2019-04-08T15:55:20Z

Hi @tyler ! Any idea when/if you'd have time to address the review comments and rebase this PR here? Or can we help you moving this forward by taking over the remaining changes?

tyler · 2019-04-10T16:58:43Z

hey @bnjbvr, yeah I can get back to this soon. I'll try to get to that this afternoon.

tyler · 2019-04-10T16:59:11Z

(If I don't, feel free to take it over. :))

… into 'brz' or 'brnz'.

…ghs.

…ting.

…R feedback.

…ends actually produce.

…ntends should produce.

…opt.

tyler · 2019-04-11T20:06:38Z

@sunfishcode or @bnjbvr, do you want to take another look before merge? I think I addressed all the things.

cranelift-codegen/src/simple_preopt.rs

bnjbvr · 2019-04-12T15:33:25Z

Thanks @tyler! I'll try to take a look at this early next week.

bnjbvr

Looks great, thanks for doing this and adding tests! I've got a few suggestions which could make the code easier to follow, let me know if they make sense to you.

cranelift-codegen/src/simple_preopt.rs

bjorn3 · 2019-04-15T14:22:56Z

I believe you meant other instead of order in the pr title :)

tyler · 2019-04-16T02:12:36Z

I believe you meant other instead of order in the pr title :)

Surprisingly, I didn't. I was referring to re-ordering branches. :)

bnjbvr · 2019-04-17T11:15:37Z

Thanks, looks great!

…f1; r=lth The one optimization introduced by this is the rearrangement of branches to prefer fallthrough whenever possible, as well as folding branches when comparing against 0. See also bytecodealliance/cranelift#629 for details. Differential Revision: https://phabricator.services.mozilla.com/D28512 --HG-- extra : rebase_source : f0d765a1cb1e2f7872037c18b9951077a08ae4b7 extra : histedit_source : 1a1dd95618e166705f7165c045f3b5af12f96d5b

…f1; r=lth The one optimization introduced by this is the rearrangement of branches to prefer fallthrough whenever possible, as well as folding branches when comparing against 0. See also bytecodealliance/cranelift#629 for details. Differential Revision: https://phabricator.services.mozilla.com/D28512 --HG-- extra : moz-landing-system : lando

…f1; r=lth The one optimization introduced by this is the rearrangement of branches to prefer fallthrough whenever possible, as well as folding branches when comparing against 0. See also bytecodealliance/cranelift#629 for details. Differential Revision: https://phabricator.services.mozilla.com/D28512

…f1; r=lth The one optimization introduced by this is the rearrangement of branches to prefer fallthrough whenever possible, as well as folding branches when comparing against 0. See also bytecodealliance/cranelift#629 for details. Differential Revision: https://phabricator.services.mozilla.com/D28512 UltraBlame original commit: 5acf68edbf599cafcf131d5fa2eb5e70c67279b8

tyler changed the title ~~Branch comparison folding and order optimizations~~ [WIP] Branch comparison folding and order optimizations Dec 21, 2018

bjorn3 suggested changes Dec 21, 2018

View reviewed changes

lib/codegen/src/simple_preopt.rs Outdated Show resolved Hide resolved

sunfishcode reviewed Dec 24, 2018

View reviewed changes

bjorn3 suggested changes Dec 25, 2018

View reviewed changes

lib/codegen/src/simple_preopt.rs Outdated Show resolved Hide resolved

tyler changed the title ~~[WIP] Branch comparison folding and order optimizations~~ Branch comparison folding and order optimizations Jan 3, 2019

tyler force-pushed the tyler/branch-opts branch from c8cc35e to 2b129f3 Compare January 3, 2019 22:03

tyler commented Jan 3, 2019

View reviewed changes

tyler force-pushed the tyler/branch-opts branch from 2b129f3 to 6d5ff80 Compare January 8, 2019 01:32

sunfishcode reviewed Jan 8, 2019

View reviewed changes

lib/codegen/src/simple_preopt.rs Outdated Show resolved Hide resolved

lib/codegen/src/simple_preopt.rs Outdated Show resolved Hide resolved

lib/codegen/src/simple_preopt.rs Outdated Show resolved Hide resolved

lib/codegen/src/simple_preopt.rs Outdated Show resolved Hide resolved

bnjbvr self-assigned this Mar 22, 2019

bnjbvr removed their assignment Apr 8, 2019

tyler added 9 commits April 10, 2019 12:52

Fold 'ifcmp_imm' + 'brif' where imm is zero and cond is 'eq' or 'ne',…

212e02f

… into 'brz' or 'brnz'.

Add note to simple_preopt differentiating its use from the preopt crate.

4f54591

Add branch order optimization to simple_preopt to encourage fallthrou…

6690cc8

…ghs.

Add tests for branch order and branch comparison folding.

caa0d4a

Questionable change: Remove domtree generation from simple_preopt tes…

3b77a47

…ting.

Refactoring of branch ordering and zero-check optimization based on P…

7edcf75

…R feedback.

Switch ifcmp+brif folding to be icmp+brz folding, which is what front…

7a8fc3d

…ends actually produce.

Switch branch order opt from brif/brff to br_icmp, as that's what fro…

7f9855a

…ntends should produce.

Add comments to branch folding and reordering functions in simple_pre…

b26d781

…opt.

tyler force-pushed the tyler/branch-opts branch from 6d5ff80 to b26d781 Compare April 10, 2019 19:54

tyler added 2 commits April 10, 2019 13:41

Address code review comments, simplifying some bits of branch_opt.

522e4d2

Switch from fallthrough back to jump.

f191cfd

tyler self-assigned this Apr 10, 2019

bjorn3 reviewed Apr 12, 2019

View reviewed changes

cranelift-codegen/src/simple_preopt.rs Outdated Show resolved Hide resolved

Remove old commented out code

16ce7c0

bnjbvr reviewed Apr 15, 2019

View reviewed changes

Style changes in response to code review.

8c0eac7

bnjbvr merged commit d43cfc6 into bytecodealliance:master Apr 17, 2019

Branch comparison folding and order optimizations #629

Branch comparison folding and order optimizations #629

Uh oh!

Conversation

tyler commented Dec 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

sunfishcode left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tyler Jan 3, 2019

Choose a reason for hiding this comment

Uh oh!

tyler commented Jan 4, 2019

Uh oh!

sunfishcode left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bnjbvr commented Apr 8, 2019

Uh oh!

tyler commented Apr 10, 2019

Uh oh!

tyler commented Apr 10, 2019

Uh oh!

tyler commented Apr 11, 2019

Uh oh!

Uh oh!

bnjbvr commented Apr 12, 2019

Uh oh!

bnjbvr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bjorn3 commented Apr 15, 2019

Uh oh!

tyler commented Apr 16, 2019

Uh oh!

bnjbvr commented Apr 17, 2019

Uh oh!

Uh oh!

tyler commented Dec 21, 2018 •

edited

Loading