[PATCH] D62703: [CVP] Simplify non-overflowing saturating add/sub

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri marked an inline comment as done.
lebedev.ri added a comment.

(can you please commit `@llvm.uadd.with.overflow.v2i32` + fix; the `@llvm.uadd.sat.v2i8` test; and the patch as three separate commits?)
Hmm, right, not sure how this should work for vectors, especially non-splat ones. Treat each line separately?



================
Comment at: llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp:414-431
 static void processOverflowIntrinsic(WithOverflowInst *WO) {
   IRBuilder<> B(WO);
   Value *NewOp = B.CreateBinOp(
       WO->getBinaryOp(), WO->getLHS(), WO->getRHS(), WO->getName());
   // Constant-holing could have happened.
   if (auto *Inst = dyn_cast<Instruction>(NewOp)) {
     if (WO->isSigned())
----------------
nikic wrote:
> lebedev.ri wrote:
> > Hm, is this not preserving debugloc?
> This does preserve debugloc. In this case it's handled by the IRBuilder API. (The overflows.ll test also does a debugify check.)
Great, thanks for checking!


================
Comment at: llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp:418
       WO->getBinaryOp(), WO->getLHS(), WO->getRHS(), WO->getName());
   // Constant-holing could have happened.
   if (auto *Inst = dyn_cast<Instruction>(NewOp)) {
----------------
lebedev.ri wrote:
> holding
err, right, `folding` (:


================
Comment at: llvm/test/Transforms/CorrelatedValuePropagation/overflows.ll:834
+;
+  %add = call { <2 x i32>, <2 x i1> } @llvm.uadd.with.overflow.v2i32(<2 x i32> %a, <2 x i32> <i32 1, i32 1>)
+  ret { <2 x i32>, <2 x i1> } %add
----------------
Hmm, great catch!
I guess saturating vectors are rare since nobody hit this yet?


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

https://reviews.llvm.org/D62703