This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] refine select-of-constants to bitwise ops
ClosedPublic

Authored by spatel on Apr 25 2018, 2:34 PM.

Details

Summary

This is an enhancement to the proposal at D45862 where we add logic for the special case when a cmp+select can clearly be reduced to just a bitwise logic instruction. The goal is to remove cases where we are not improving the IR instruction count when doing these select transforms, and in all cases here that is true.

I think this will have the same results on the 3-way compare tests that are proposed in the other patch. We should commit those first, so we can see the diffs here or in the other patch.

I looked some at x86 and AArch64 codegen with this change, and we're going to get more cmov/vblend and csel/bsl because the DAG doesn't have replacements for all of these folds. In some cases, the changes look better and others it seems worse, but I suspect we'll have to run detailed perf tests per uarch to know the true perf outcome.

Diff Detail

Event Timeline

spatel created this revision.Apr 25 2018, 2:34 PM

The bit hackery isn't clear to me even after staring at this all day. Here's the Alive link I posted earlier in case it helps:
https://rise4fun.com/Alive/XG7
Suggestions to improve the comments and code welcome.

Also, the tests reveal a couple of simpler but non-obvious folds that we seem to be missing:
https://rise4fun.com/Alive/AeL

Name: add_to_xor
%a = and i8 %x, 128
%r = add i8 %a, 131
=>
%r = xor i8 %a, 131

Name: orxor
%a = or i8 %x, -9
%r = xor i8 %a, 8
=>
%a1 = xor i8 %x, -1
%r = or i8 %a1, -9
spatel updated this revision to Diff 144446.Apr 28 2018, 7:39 AM

Patch updated:
Rebased to include 3-way compare tests added at rL331100.

spatel added a comment.May 2 2018, 3:33 PM

Ping.

This patch improves the motivating cases (3-way-compare) without the regressions in D45862.

This doesn't preclude more canonicalization to select, but if we want any more select IR than this, then I think we must add folds to the DAGCombiner first.

This revision is now accepted and ready to land.May 2 2018, 6:43 PM
This revision was automatically updated to reflect the committed changes.