[PATCH] D38494: [SCEV] Handling for ICmp occuring in the evolution chain.

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy accepted this revision.
sanjoy added a comment.
This revision is now accepted and ready to land.

Mostly minor stuff; please check in directly after addressing these.



================
Comment at: lib/Analysis/ScalarEvolution.cpp:4099
+        IsPosBECond = BI->getSuccessor(0) == L->getHeader();
+      } else
+        return S;
----------------
It is usually customary to to put the `else` block in braces if the `if` is in braces.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:4108
+    const SCEV *Result = Expr;
+    bool InvariantF = SE.isLoopInvariant(Expr, L);
+
----------------
No need to change it in this patch, but this constraint isn't necessary, is it?


================
Comment at: lib/Analysis/ScalarEvolution.cpp:4149
+
+Optional<const SCEV *>
+SCEVBackedgeConditionFolder::compareWithBackedgeCondition(Value *IC) {
----------------
I think it is better to return `Optional<bool>` here, and instead call `getOne` and `getZero` in the `default:` case in the switch on `I->getOpcode()` which is the only place that needs it.  It will also make the `case Instruction::Select:` code a bit simpler.


================
Comment at: test/Analysis/ScalarEvolution/pr34538.ll:32
+  %inc.start.0 = add nsw i32 %start.0, %select_ext
+  br i1 %cmp, label %do.body, label %do.end
+
----------------
Please also add a test case where both the latch branches branch to the loop header.


https://reviews.llvm.org/D38494