[PATCH] D21725: [LoopUnswitch] Unswitch on conditions feeding into guards

David Majnemer via llvm-commits llvm-commits at lists.llvm.org
Sat Jun 25 01:17:08 PDT 2016


majnemer added inline comments.

================
Comment at: lib/Transforms/Scalar/LoopUnswitch.cpp:518
@@ -516,6 +517,3 @@
 
-  // Do not unswitch loops containing convergent operations, as we might be
-  // making them control dependent on the unswitch value when they were not
-  // before.
-  // FIXME: This could be refined to only bail if the convergent operation is
-  // not already control-dependent on the unswitch value.
+  // Run through the instructions in the loop, keeping track of three things
+  //
----------------
For some reason my eye is hunting for punctuation, maybe throw on a ':'

================
Comment at: lib/Transforms/Scalar/LoopUnswitch.cpp:526
@@ +525,3 @@
+  //
+  //  - That not loop blocks contain invokes whose predecessor edges we cannot
+  //    split
----------------
Perhaps "That loops don't contain ..."

================
Comment at: lib/Transforms/Scalar/LoopUnswitch.cpp:527
@@ +526,3 @@
+  //  - That not loop blocks contain invokes whose predecessor edges we cannot
+  //    split
+  //
----------------
.

================
Comment at: lib/Transforms/Scalar/LoopUnswitch.cpp:532
@@ +531,3 @@
+
+  SmallVector<AssertingVH<IntrinsicInst>, 4> Guards;
+
----------------
Is it safe to make this an AssertingVH?  Could unswitching one guard lead to another guard getting cloned while the original gets erased?


http://reviews.llvm.org/D21725





More information about the llvm-commits mailing list