This is an archive of the discontinued LLVM Phabricator instance.

Constant fold switch inst when looking for trivial conditions to unswitch on.
ClosedPublic

Authored by trentxintong on Jan 23 2017, 9:43 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

trentxintong created this revision.Jan 23 2017, 9:43 AM
efriedma added inline comments.Jan 23 2017, 12:30 PM
lib/Transforms/Scalar/LoopUnswitch.cpp
922 ↗(On Diff #85417)

Is this likely to actually do anything in practice...? Most passes (including LoopUnswitch itself) will constant-fold newly generated code.

928 ↗(On Diff #85417)

Checking for a Constant isn't good enough here: you need a ConstantInt.

trentxintong added inline comments.Jan 23 2017, 2:23 PM
lib/Transforms/Scalar/LoopUnswitch.cpp
922 ↗(On Diff #85417)

I agree that the constants probably have been folded at this point. The only constants remaining are likely to be the ones generated by unswitching (Even those ones are likely to be folded away by SimplifyCode in LoopUnswitch) or maybe we have some pass ordering problems.

I am fine dropping the ConstantFoldInstruction, as this does add some compilation time to the pass.

928 ↗(On Diff #85417)

Yes, this has to be a constant int of abitrary width.

Address comments.

After thinking about it. I decided to drop the constant-folding as they are not trivially cheap.
Especially at this point, they have probably been folded away, the ones remaining probably can
not be folded anyways.

@efriedma Does this look OK now ? Thanks.

efriedma added inline comments.Jan 26 2017, 2:58 PM
lib/Transforms/Scalar/LoopUnswitch.cpp
931 ↗(On Diff #85458)

Can you use SwitchInst::findCaseValue rather than an explicit loop?

Address comments.

efriedma accepted this revision.Jan 26 2017, 5:45 PM

LGTM.

This revision is now accepted and ready to land.Jan 26 2017, 5:45 PM
This revision was automatically updated to reflect the committed changes.

@efriedma Thanks for the review!