[PATCH] D75011: [InstCombine] Remove trivially empty ranges from end

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:1482
+    std::function<bool(const IntrinsicInst &)> IsStart) {
+  BasicBlock::reverse_iterator BI(EndI), BE(EndI.getParent()->rend());
   for (++BI; BI != BE; ++BI) {
----------------
Add a comment to explain why we're using the end instruction and reverse_iterator (can copy/adapt text from this patch description).


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:1488-1489
+        continue;
+      if (IsStart(*I)) {
+        if (haveSameOperands(EndI, *I, EndI.getNumArgOperands())) {
+          IC.eraseInstFromFunction(*I);
----------------
Is there a logic change from 'if &&' to 'if, if ...continue'? If so, that deserves a code comment. 

If it's just a matter of taste, it would be better to make that + a partial function signature change as a preliminary NFC commit. (Trying to limit patch risk here because I'm not very familiar with these transforms.)


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

https://reviews.llvm.org/D75011