[PATCH] D41944: [LLVM][IR][LIT] support of 'no-overflow' flag for sdiv\udiv instructions

[Mohammed Agabaria via Phabricator via llvm-commits]

magabari marked 7 inline comments as done.
magabari added a comment.

fixed craig and eli notes



================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:574
+/// replace the upsupported division with a scalar safe code
+bool CodeGenPrepare::fixMayOverflowIntegerDiv(Instruction &I, bool isSigned) {
+  assert((I.getOpcode() == Instruction::SDiv ||
----------------
efriedma wrote:
> CodeGenPrepare doesn't run at -O0; you'll have to put this code somewhere else.
added new pass "ScalarizeMayOverflowDiv"


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:647
+  Value *VResult = UndefVal;
+  Value *PrevPhi = UndefVal;
+  unsigned VectorWidth = I.getType()->getVectorNumElements();
----------------
craig.topper wrote:
> magabari wrote:
> > craig.topper wrote:
> > > Is the starting value of PrevPhi ever expected to be used? Does it need to be undef?
> > PrevPhi will be used at the first merge "undef" i think you meant VResult which can be uninitialized.
> Isn't VResult consumed by the CreateInsertElement on the first iteration of the loop on line 731?
i think both can't be uninitialized.
VResult used in CreateInsertElement
PrevPhi used in Phi->addIncoming


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:625
+          ConstantInt::get(
+              B->getType(),
+              MinVal));
----------------
craig.topper wrote:
> Why are all the arguments to ConstantInt::get on separate lines? It looks short enough for one line.
created a new pass "scalarizeMayOverflowDiv" and passed clang-format


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:690
+/*  
+  SmallVector<Constant *, 4> ZeroConstants;
+  SmallVector<Constant *, 4> MinusOneConstants;
----------------
craig.topper wrote:
> Remove commented out code.
Sorry my fault


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:718
+
+  for (unsigned Idx = 0; Idx < VectorWidth; ++Idx) {
+    if (Idx > 0) {
----------------
craig.topper wrote:
> What test file covers this code? I tried to look for one, but there are a lot of test updates and all I found was adding 'nof' to existing tests.
added "scalarize-may-overflow-div.ll"


https://reviews.llvm.org/D41944