This is an archive of the discontinued LLVM Phabricator instance.

[GlobalISel] Implement bit-test switch table optimization
ClosedPublic

Authored by aemerson on Aug 4 2020, 11:29 AM.

Details

Summary

This is mostly a straight port from SelectionDAG. We re-use the actual bit-test analysis part from SwitchLoweringUtils, which was factored out earlier to support jump-tables.

Diff Detail

Event Timeline

aemerson created this revision.Aug 4 2020, 11:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 4 2020, 11:29 AM
aemerson requested review of this revision.Aug 4 2020, 11:29 AM
arsenm added inline comments.Aug 4 2020, 11:38 AM
llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
729

Should just change the insert point for the existing one? This loses the CSEInfo

738

Don't need the {}

744

Why depend on "legal" types? The concept should mostly be going away

757

This loses the address space, and I'm not sure why you would ever be avoiding the pointer type. Also, in general you should never need to call getPointerTy, since you can just re-use the type of the thing you already have

776

Don't need getReg

790

Same as above

795

Don't need = 0

940

Missing newline

paquette added inline comments.Aug 4 2020, 12:40 PM
llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
784

Move Doxygen comment to IRTranslator.h?

Also this comment isn't super useful.

aemerson added inline comments.Aug 4 2020, 1:00 PM
llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
744

This was from SelectionDAG, I think for performance reasons. I'll remove it.

776

Actually I do for buildBrCond() since that is still expecting a Register. I'll change that though.

aemerson updated this revision to Diff 283017.Aug 4 2020, 1:39 PM

Address feedback.

aemerson updated this revision to Diff 283018.Aug 4 2020, 1:40 PM

Posted old patch before, address feedback for reals this time.

Harbormaster completed remote builds in B66985: Diff 283018.
arsenm added inline comments.Aug 4 2020, 2:31 PM
llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
769

Still have the .getReg not that it really matters

aemerson added inline comments.Aug 4 2020, 2:55 PM
llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
769

I'll remove it before commit.

arsenm added inline comments.Aug 11 2020, 7:52 AM
llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-switch-bittest.ll
133

Can you add a few degenerate cases with 1 and 2 switch cases (and 0 if that's even accepted).

I also don't think any of these hit the omit-branch-to-next block case

aemerson added inline comments.Aug 11 2020, 10:33 AM
llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-switch-bittest.ll
133

I'll add a test to check for a single BT cluster, but 1 or 2 switch cases by themselves won't trigger this optimization, it'll just use a simple equality check.

aemerson updated this revision to Diff 284820.Aug 11 2020, 10:58 AM

Add another test case for just a single BT cluster. This test also falls through instead of generating a G_BR.

arsenm accepted this revision.Aug 11 2020, 11:37 AM
This revision is now accepted and ready to land.Aug 11 2020, 11:37 AM
This revision was landed with ongoing or failed builds.Aug 12 2020, 11:32 AM
This revision was automatically updated to reflect the committed changes.