[PATCH] D50499: [X86] Constant folding of adds/subs intrinsics

Craig Topper via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 9 10:04:33 PDT 2018


craig.topper added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:265
+    break;
+  case Intrinsic::x86_avx512_mask_padds_b_512:
+  case Intrinsic::x86_avx512_mask_padds_w_512:
----------------
We should really remove masking from these intrinsics and use a select. But that can be a follow up.


================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:286
+
+  APInt MaxValue = APInt::getSignedMaxValue(SVT->getPrimitiveSizeInBits());
+  APInt MinValue = APInt::getSignedMinValue(SVT->getPrimitiveSizeInBits());
----------------
Probably could use getIntegerBitWidth instead of getPrimitiveSizeInBits. Should be slightly cheaper since getPrimitiveSizeInBits has to detect that its an integer and then call getIntegerBitWidth.


================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:296
+
+    APInt Val0 = cast<ConstantInt>(Elt0)->getValue();
+    APInt Val1 = cast<ConstantInt>(Elt1)->getValue();
----------------
const APInt & Val0


================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:296
+
+    APInt Val0 = cast<ConstantInt>(Elt0)->getValue();
+    APInt Val1 = cast<ConstantInt>(Elt1)->getValue();
----------------
craig.topper wrote:
> const APInt & Val0
Isn't it possible for the element to be a ConstantExpr? In which case this case would fail


Repository:
  rL LLVM

https://reviews.llvm.org/D50499





More information about the llvm-commits mailing list