This is an archive of the discontinued LLVM Phabricator instance.

[SimpleLoopUnswitch] Make all checks before actual non-trivial unswitch
ClosedPublic

Authored by mkazantsev on Oct 26 2018, 1:46 AM.

Details

Summary

We should be able to make all relevant checks before we actually start the non-trivial
unswitching, so that we could guarantee that once we have started the transform,
it will always succeed.

Diff Detail

Event Timeline

mkazantsev created this revision.Oct 26 2018, 1:46 AM

FWIW, after my suggestion I would not call this "NFC" as it should change the cost function of this routine at least....

lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
2368–2376

Can we hoist this even fiurther up? Maybe w/ the irreducible CFG?

We shouldn't even compute and rank the cost of the candidates when this is the case.

The reason this was inside the unswitch (ages ago) was because we were re-using teh exit blocks list and I didn't want to create it twice, but your solution of passing it in seems way better.

mkazantsev marked an inline comment as done.
mkazantsev retitled this revision from [NFC][SimpleLoopUnswitch] Make all checks before actual non-trivial unswitch to [SimpleLoopUnswitch] Make all checks before actual non-trivial unswitch.Oct 26 2018, 2:24 AM
chandlerc accepted this revision.Oct 26 2018, 2:28 AM

Awesome improvement, thanks!

LGTM. Minor tweak or follow-up suggested below.

lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
1799

Maybe make this come before the dom tree? I'd generally prefer roughly grouped params... but SCEV is already a bit out-of-order, so also fine to clean up the grouping of parameters in a follow-up, whatever is easiest. I think the best ordering here would be: loop, terminator, invarianst, exit blocks, callback, analyses-used.

This revision is now accepted and ready to land.Oct 26 2018, 2:28 AM
mkazantsev marked an inline comment as done.Oct 26 2018, 2:49 AM
This revision was automatically updated to reflect the committed changes.