[PATCH] D93764: [LoopUnswitch] Implement first version of partial unswitching.

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked an inline comment as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopUnswitch.cpp:671
+  SmallVector<Value *, 4> WorkList;
+  WorkList.append(CondI->op_begin(), CondI->op_end());
+
----------------
mdchen wrote:
> I guess only the conditional value needs to be added here.
Yes that would be sufficient, if the loop below would support compares, which it does not in the current version. It would be good to extend the supported instructions below to any arithmetic/compare instruction in the future though.


================
Comment at: llvm/lib/Transforms/Scalar/LoopUnswitch.cpp:692
+      // Queue the defining access to check for alias checks.
+      AccessesToCheck.push_back(MemUse->getDefiningAccess());
+      AccessedLocs.push_back(MemoryLocation::get(I));
----------------
mdchen wrote:
> `MemUse` could be nullptr.
I think we should not reach the code, if `nullptr` is assigned to `MemUse` in the `if` condition.


================
Comment at: llvm/lib/Transforms/Scalar/LoopUnswitch.cpp:713
+          BasicBlock *Current = WorkList.back();
+          WorkList.pop_back();
+          if (!L->contains(Current))
----------------
mdchen wrote:
> pop_back_val()
Thanks, I updated the code!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93764