[PATCH] D46706: [PM/LoopUnswitch] Support partial trivial unswitching.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

Thanks for the review!



================
Comment at: llvm/test/Transforms/SimpleLoopUnswitch/trivial-unswitch.ll:511
+  %cond_or4 = or i1 %cond_or3, %cond4
+  br i1 %cond_or4, label %loop_exit, label %do_something
+; CHECK:       loop_begin:
----------------
sanjoy wrote:
> I think we need to test a few more cases here:
> 
>  - Branch on chain of `and`s
>  - Branch on mix of `and`s and `or`s and possibly other operations
So, we test branching on an `and` above. There is no different logic in handling N `and` instructions vs. N `or` instructions, so I didn't add that test as it didn't seem to add value compared to these two.

I can definitely mix some non `or` operations into this chain. Would that be enough coverage? Just trying to understand what the goal is of the added tests.

While I was adding these tests, it actually exposed a weakness in the instruction simplification we do here. Fixing it proved... a bit tricky. I've added somewhat complex logic so that when we need to simplify multiple different invariants we can correctly simplify defs before uses. However, now I need to add a better test to cover the nasty case for simplifying -- when we need to simplify around a diamond in the CFG. I'm going ahead and uploading the patch while I craft a test case for that so you can yell if I'm making this way harder than it needs to be.


Repository:
  rL LLVM

https://reviews.llvm.org/D46706