[PATCH] D47522: [PM/LoopUnswitch] Add partial non-trivial unswitching for invariant conditions feeding a chain of `and`s or `or`s for a branch.

Chandler Carruth via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 30 02:34:04 PDT 2018


chandlerc created this revision.
chandlerc added reviewers: asbirlea, fedor.sergeev, sanjoy.
Herald added subscribers: hiraditya, mcrosier.

Much like with full non-trivial unswitching, we rely on the pass manager to
handle iterating until all of the profitable unswitches have been done. This is
to allow other more profitable unswitches to fire on any of the cloned, simpler
versions of the loop if viable.

Threading the partial unswiching through the non-trivial unswitching logic
motivated some minor refactorings. If those are too disruptive to make it
reasonable to review this patch, I can separate them out, but it'll be somewhat
timeconsuming so I wanted to send it for initial review as-is. Feel free to
tell me whether it warrants pulling apart.

I've tried to re-use (and factor out) logic form the partial trivial
unswitching, but not as much could be shared as I had haped. Still, this wasn't
as bad as I naively expected.

Some basic testing is added, but I probably need more. Suggestions for things
you'd like to see tested more than welcome. One thing I'd like to do is add
some testing that when we schedule this with loop-instsimplify it effectively
cleans up the cruft created.

Last but not least, this uncovered a bug that has been in loop cloning the
entire time for non-trivial unswitching. Specifically, we didn't correctly add
the outer-most cloned loop to the list of cloned loops. This meant that LCSSA
wouldn't be updated for it hypothetically, and more significantly that we would
never visit it in the loop pass manager. I noticed this while checking
loop-instsimplify by hand. I'll try to separate this bugfix out into its own
patch with a more focused test. But it is just one line, so shouldn't
significantly confuse the review here.

Depends on https://reviews.llvm.org/D46706 for the trivial case.

After this patch, the only missing "feature" in this unswitch I'm aware of us
non-trivial unswitching of switches. I'll try implementing *full* non-trivial
unswitching of switches (which is at least a sound thing to implement), but
*partial* non-trivial unswitching of switches is something I don't see any
sound and principled way to implement. I also have no interesting test cases
for the latter, so I'm not really worried. The rest of the things that need to
be ported are bug-fixes and more narrow / targeted support for specific issues.


Repository:
  rL LLVM

https://reviews.llvm.org/D47522

Files:
  llvm/include/llvm/ADT/TinyPtrVector.h
  llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
  llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D47522.149067.patch
Type: text/x-patch
Size: 26608 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180530/1e596411/attachment-0001.bin>


More information about the llvm-commits mailing list