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

[SAHIL GIRISH YERAWAR via Phabricator via llvm-commits]

cs15btech11044 added inline comments.


================
Comment at: lib/Support/ScopHelper.cpp:461
+
+    for (unsigned int Idx = 1; Idx < GepInst->getNumIndices(); Idx++) {
+      Value *Val = GepInst->getOperand(Idx);
----------------
Meinersbur wrote:
> [style] [[ https://llvm.org/docs/CodingStandards.html#don-t-evaluate-end-every-time-through-a-loop | Do not evaluate something invariant in the loop exit condition ]]
> 
> [serious] Off-by-one-error: `getNumIndices()` returns `getNumOperands() - 1`, so you are skipping the last element here.
> 
> [style] Could use a foreach-loop: `for (const Use &Val : llvm::drop_begin(this->operands(), 1))`, but this could require D48598.
If the foreach loop requires `D48598`, I would wait till it gets committed to LLVM repository. This change could be taken up in the tentative follow-up patch.


https://reviews.llvm.org/D48026