[PATCH] D97756: [LoopUnswitch] unswitch if cond is in select form of and/or as well

[Nikita Popov via Phabricator via llvm-commits]

nikic accepted this revision.
nikic added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:145
 
-      // Visit this operand.
-      if (Visited.insert(OpI).second)
-        Worklist.push_back(OpI);
+      if (OpI && (OpI->getOpcode() == Root.getOpcode() ||
+                  (IsRootAnd && match(OpI, m_LogicalAnd())) ||
----------------
I don't think the opcode check is needed here.


================
Comment at: llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch.ll:4212
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    %[[INV_AND:.*]] = and i1 %cond2, %cond1
+; CHECK-NEXT:    br i1 %[[INV_AND]], label %entry.split, label %entry.split.us
----------------
aqjune wrote:
> nikic wrote:
> > This doesn't look safe. Previously, if `%v1` is false then `%cond1` is not used. Now we unconditionally branch on `%cond1`.
> > 
> > Or is the logic here that unswitching requires insertion of `freeze` for normal and/or anyway, and we leave this as a miscompile until that is resolved?
> Yes, I think the issue can resolved in a separate patch; anyway loop unswitch requires insertion of `freeze`.
Okay, that's fine by me. We did the same for the SimplifyCFG switch transform.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97756/new/

https://reviews.llvm.org/D97756