[PATCH] D67162: [InstCombine] Known-bits optimization for ARM MVE VADC.

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Sounds good.

We may also be able to do something similar in ISel, during the optimisation there, but this looks like a more general fix (and I'm not sure that would add a lot over what is already here).



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:3299
   }
+  case Intrinsic::arm_mve_minv_u:
+  case Intrinsic::arm_mve_minv_s: {
----------------
This looks like something else entirely!

It's setting the range metadata on vminv's?


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:3334
+    Value *CarryArg = II->getArgOperand(CarryOp);
+    unsigned CarryWidth = CarryArg->getType()->getScalarSizeInBits();
+
----------------
Is this ever not 32? If so the 32 below should be the same value


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:3338
+    if (SimplifyDemandedBits(II, CarryOp, APInt::getOneBitSet(CarryWidth, 29),
+                             CarryKnown, 0))
+      return II;
----------------
I think the last parameter is depth, and the default is 0? If so it might as well be left off.


================
Comment at: llvm/test/CodeGen/Thumb2/mve-intrinsics/vadc-multiple.ll:20
+  %fpscr.in.1 = shl i32 %carry.out.0, 29
+  %outpair.1 = call { <4 x i32>, i32 } @llvm.arm.mve.vadc.v4i32(<4 x i32> %a.1, <4 x i32> %b.1, i32 %fpscr.in.1)
+  %fpscr.out.1 = extractvalue { <4 x i32>, i32 } %outpair.1, 1
----------------
Can you include a test for arm_mve_vadc_predicated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67162