[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/AsmParser/LLParser.cpp:3195
+      if (Opc == Instruction::SDiv || Opc == Instruction::UDiv) {
+        if (EatIfPresent(lltok::kw_nof)) {
+          NOF = true;
----------------
What happens if 'nof' and 'mof' are both present?


================
Comment at: lib/CodeGen/ScalarizeMayOverflowDiv.cpp:220
+  Type *ElementTy = I.getType()->getVectorElementType();
+  Value *ZeroVector = ConstantInt::get(I.getType(), 0, false);
+  Value *MinusOneVector = ConstantInt::get(I.getType(), -1, true);
----------------
Drop the "false".


================
Comment at: lib/CodeGen/ScalarizeMayOverflowDiv.cpp:221
+  Value *ZeroVector = ConstantInt::get(I.getType(), 0, false);
+  Value *MinusOneVector = ConstantInt::get(I.getType(), -1, true);
+  Value *SignedMinVector = ConstantInt::get(
----------------
Use "getSigned" and drop the true.


================
Comment at: lib/CodeGen/ScalarizeMayOverflowDiv.cpp:224
+      I.getType(),
+      APInt::getSignedMinValue(
+                                      ElementTy->getIntegerBitWidth()));
----------------
Weird formatting here.


================
Comment at: lib/CodeGen/ScalarizeMayOverflowDiv.cpp:265
+  Phi->addIncoming(VResult, CondBlock);
+  Phi->addIncoming(PrevPhi, PrevIfBlock);
+  I.replaceAllUsesWith(Phi);
----------------
If the vector only has 1 element, PrevPhi is undef. But that's not correct is it?


================
Comment at: test/Transforms/ScalarizeMayOverflowDiv/scalarize-may-overflow-div.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt %s -mcpu=atom -scalarize-may-overflow-div -S -o - | FileCheck %s --check-prefix=ATOM
----------------
Add a test case for a vector with only 1 element. i.e. <1 x i32>. That's the case where PrevPHI would be undef after the loop right?


https://reviews.llvm.org/D41944