[PATCH] D137632: [LoopPredication] Widen checks if condition operands constant ranges are known

[Artur Pilipenko via Phabricator via llvm-commits]

apilipenko added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopPredication.cpp:248-249
 
+static cl::opt<bool> WidenChecksUsingConstantRange(
+    "loop-predication-widen-checks-using-contants-range", cl::Hidden,
+    cl::desc(
----------------



================
Comment at: llvm/lib/Transforms/Scalar/LoopPredication.cpp:404-405
+
+  auto &DL = L.getHeader()->getModule()->getDataLayout();
+  LazyValueInfo LVI(&AR.AC, &DL, &AR.TLI);
+
----------------
This is a non-canonical way to get an analysis. This way it will not benefit from analysis manager caching. I guess you have to do this because LVI is a function analysis but a part of LoopStandardAnalysisResults. 

I'm not sure how we should go about it, but I suggest splitting the LVI off from the initial patch. You can still get the constant range from ValueTracking to get the basic version of this optimization.


================
Comment at: llvm/lib/Transforms/Scalar/LoopPredication.cpp:817-831
+  switch (Pred) {
+  case CmpInst::ICMP_EQ:
+  case CmpInst::ICMP_NE:
+    LLVM_DEBUG(dbgs() << "Unsupported predicate\n");
+    return None;
+  case CmpInst::ICMP_UGT:
+  case CmpInst::ICMP_UGE:
----------------
Please, spell the list of supported predicated explicitly here. 


================
Comment at: llvm/lib/Transforms/Scalar/LoopPredication.cpp:850
+
+  bool LHSMaxIsGreaterThanRHSMax = LHSMaxValue.ugt(RHSMaxValue);
+  if (IsSigned)
----------------
Why do you need to check this?


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

https://reviews.llvm.org/D137632