[PATCH] D132208: [LoopIntWrapPredication] Loop Integer Wrapping Predication Pass

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopIntWrapPredication.cpp:70
+  if (Opc != Instruction::Add && Opc != Instruction::Sub &&
+      Opc != Instruction::Mul)
+    return nullptr;
----------------
The description on line 32 mentions `shl`, but `Shl` isn't here


================
Comment at: llvm/lib/Transforms/Scalar/LoopIntWrapPredication.cpp:246
+  BasicBlock *Preheader = L.getLoopPreheader();
+  assert(Preheader);
+  Instruction *Loc = Preheader->getTerminator();
----------------
What guarantees the loop has a preheader? Placement in the pipeline would probably guarantee it but I'm not sure anything does when running the pass by itself. Unless I missed something.


================
Comment at: llvm/lib/Transforms/Scalar/LoopIntWrapPredication.cpp:248
+  Instruction *Loc = Preheader->getTerminator();
+  assert(Loc);
+  SmallVector<ArithmeticChain, 4> Chains;
----------------
This assert is probably unnecessary. A terminator is required for the Preheader to even be recognized as a predecessor of the loop.


================
Comment at: llvm/lib/Transforms/Scalar/LoopIntWrapPredication.cpp:275
+  }
+  // Mark loop to not proceed it again.
+  addStringMetadataToLoop(&L, LoopIntWrapPredicationMetadata);
----------------
proceed -> process?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132208