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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:647
+  Value *VResult = UndefVal;
+  Value *PrevPhi = UndefVal;
+  unsigned VectorWidth = I.getType()->getVectorNumElements();
----------------
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?


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:625
+          ConstantInt::get(
+              B->getType(),
+              MinVal));
----------------
Why are all the arguments to ConstantInt::get on separate lines? It looks short enough for one line.


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


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:703
+*/  
+  Value *ZeroVector      = ConstantVector::getSplat(VectorWidth, 
+                                             ConstantInt::get(ElementTy, 0));
----------------
I should of caught this early, but if you pass a vector type to ConstantInt::get it will automatically create a splat. So you don't even need to call ConstantVector::getSplat you just need to pass I.getType() to ConstantInt::get


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:718
+
+  for (unsigned Idx = 0; Idx < VectorWidth; ++Idx) {
+    if (Idx > 0) {
----------------
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.


https://reviews.llvm.org/D41944