[PATCH] D60668: [IR] Add WithOverflowInst class

[Nikita Popov via Phabricator via llvm-commits]

nikic marked 3 inline comments as done.
nikic added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:4584
+    bool Signed = WO->isSigned();
+    // TODO: Should add nuw/nsw flags for mul as well.
+    if (BinOp == Instruction::Mul || !isOverflowIntrinsicNoWrap(WO, DT))
----------------
lebedev.ri wrote:
> If you don't intend to look into that, could you please file a bug, if there isn't one already?
Just looked into this one... The NUW/NSW flags set here are actually only used when constructing AddRecs, which of course doesn't apply to multiplies. So I think I'll just replace this TODO with a comment saying so.


================
Comment at: llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp:403
+static bool willNotOverflow(WithOverflowInst *WO, LazyValueInfo *LVI) {
+  // TODO: Also support multiplication.
+  Instruction::BinaryOps BinOp = WO->getBinaryOp();
----------------
lebedev.ri wrote:
> Similarly, if there is no bug yet, could you please create one?
For reference, this is D60791.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60668