[PATCH] D60565: [LOOPINFO] Extend Loop object to add utilities to get the loop bounds, step, induction variable, and guard branch.

[Ettore Tiotto via Phabricator via llvm-commits]

etiotto added inline comments.


================
Comment at: llvm/lib/Analysis/LoopInfo.cpp:172
+  ConstantInt *C2 = dyn_cast<ConstantInt>(&V2);
+  if (C1 != nullptr && C2 != nullptr) {
+    if (C1->getSExtValue() == C2->getSExtValue())
----------------
Whitney wrote:
> fhahn wrote:
> > LLVM coding convention is not to check for nullptr explicitly, just use if (C1 && C2) (here and below)
> Didn't know this coding convention, will remove the nullptr checks.
Can you point out the section in https://llvm.org/docs/CodingStandards.html that mentions that? I did a quick scan and I could not find a reference. Much of the existing code in LLVM also use nullptr in if conditional expressions. 


================
Comment at: llvm/lib/Analysis/LoopInfo.cpp:191
+  // Simplify(V1) == Simplify(V2)
+  if (Instruction *I1 = dyn_cast<Instruction>(&V1))
+    if (Value *V = SimplifyInstruction(I1, I1->getModule()->getDataLayout()))
----------------
Whitney wrote:
> fhahn wrote:
> > I'm not sure we should use SimplifyInstruction here. Ideally everything should be already simplified?
> I agree that ideally everything should be already simplified, but I encounter cases which the instructions are not simplified, and I don't think we can assume the instructions are simplified or not, as getGuard() can be called in any point in time.
It depends. Is feasible a consumer may not have simplified the expression prior to calling this member function. So, arguably, is useful to attempt to simplify it here.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60565