[PATCH] D48026: [ScopHelper] Provide support for recognising collective invariant loads

[Philip Pfaffe via Phabricator via llvm-commits]

philip.pfaffe added inline comments.


================
Comment at: lib/Analysis/ScopDetection.cpp:1168
+              invariantLS.insert(Load);
+              break;
+            } else {
----------------
cs15btech11044 wrote:
> philip.pfaffe wrote:
> > Why do you break on the first invariant load? Why not mark all hoistables in one go?
> As soon as it is decided that a load is invariant, it is put in a set and we restart this process. This would ensure invariant loads which are collective(i.e a load depending on a previous invariant load instruction), if they appear out-of-order during iteration, we still detect it as invariant.
The early break causes the loop to restart every time an invariant load is detected, even if loads are _not_ collective.


================
Comment at: lib/Support/ScopHelper.cpp:450
+
+  auto *ptr_operand = dyn_cast<Instruction>(Ptr);
+  if (ptr_operand && isa<GetElementPtrInst>(ptr_operand)) {
----------------
Meinersbur wrote:
> [style] Please name variable according to the [[ https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly | LLVM coding standard ]].
> 
> Consider adding a comment about what this code is doing.
Variable names should be ProudCamelCase.


https://reviews.llvm.org/D48026