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

[Philip Pfaffe via Phabricator via llvm-commits]

philip.pfaffe added a comment.

This is starting to look good! Only cosmetic nits left.

The testcase is still missing though.



================
Comment at: lib/Analysis/ScopDetection.cpp:1170
+              InvariantLS.insert(Load);
+              // break;
+            } else {
----------------
This should be removed.


================
Comment at: lib/Support/ScopHelper.cpp:446
+                            ScalarEvolution &SE, const DominatorTree &DT,
+                            InvariantLoadsSetTy &RequiredILS) {
   Loop *L = LI.getLoopFor(LInst->getParent());
----------------
RequiredILS should be `const&`.

More bikeshed: Why 'Required'? I'd consider 'KnownInvariantLoads' to be more apt, what do you think?


https://reviews.llvm.org/D48026