[PATCH] D55232: [SCEV] Suppress hoisting insertion point of binops when unsafe

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy requested changes to this revision.
sanjoy added a comment.
This revision now requires changes to proceed.

Only minor nitpicky comments, will LGTM once these are fixed.

Normally I'd ask you to wait for Max, but looks like he is no longer at Azul and thus is not getting phabricator emails.



================
Comment at: include/llvm/Analysis/ScalarEvolutionExpander.h:321
+    Value *InsertBinop(Instruction::BinaryOps Opcode, Value *LHS, Value *RHS,
+                       bool IsSafeToHoist = true);
 
----------------
I'd slightly prefer to not default this to anything, is that feasible?


================
Comment at: test/Transforms/LoopVectorize/pr38697.ll:202
+  %arrayidx6 = getelementptr inbounds i32, i32* %ptr, i64 %idxprom5
+  %1 = load i32, i32* %arrayidx6, align 4
+  %add7 = add i32 %1, %val.126
----------------
We should give these instructions names so that updating the tests remains easy (use `opt -instnamer`).


================
Comment at: test/Transforms/LoopVectorize/pr38697.ll:228
+; right-shift ('lshr') rather than an explicit division.
+; loop body.
+; INDVARCHECK-LABEL: DivBy16Hoist
----------------
Can you please make these CHECK lines a bit more specific?  Maybe using `update_test_checks.py`?


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

https://reviews.llvm.org/D55232