[PATCH] D99354: [SimpleLoopUnswitch] Port partially invariant unswitch from LoopUnswitch to SimpleLoopUnswitch

[Florian Hahn via Phabricator via llvm-commits]

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

LGTM, thanks! A few small comments that can be addressed before committing without further review I think.



================
Comment at: llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:3153
+    if (CurrentLoopValid) {
+      // If the current loop has been unswitched with partially invariant
+      // condition, we should not re-add the current loop to avoid unswitching
----------------
nit: perhaps `unswitched using a partially invariant condition, ...`?


================
Comment at: llvm/test/Transforms/SimpleLoopUnswitch/partial-unswitch.ll:1104
+
+define void @test(i32 %0, i32 %1, i32 %2, ...) {
+; CHECK-LABEL: @test(
----------------
Could you add a brief comment and a more descriptive name for the test?


================
Comment at: llvm/test/Transforms/SimpleLoopUnswitch/partial-unswitch.ll:1143
+  br label %5
+
+5:
----------------
It would be great if you could use descriptive labels in the test both for basic blocks and values, to make it easier to read in case people need to take a look in the future.


================
Comment at: llvm/test/Transforms/SimpleLoopUnswitch/partial-unswitch.ll:1150
+8:
+  store i32 undef, i32* null, align 16
+  br label %9
----------------
please avoid using `null` as pointer, as this UB


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

https://reviews.llvm.org/D99354