[PatternMatch] Allow poison in api_pred_ty matchers #89188

nikic · 2024-04-18T08:49:55Z

This allows vector splats containing poison for matchers using api_pred_ty, making the behavior consistent with cst_pred_ty. In other words, previously m_NonNegative() allowed splats with poison, while m_NonNegative(C) did not. Now both allow them.

The affected matchers are m_MaxSignedValue, m_Negative, m_NonNegative, m_StrictlyPositive, m_NonPositive, m_Power2, m_Power2OrZero, m_NegatedPower2, m_NegatedPower2OrZero, m_LowBitMask and m_LowBitMaskOrZero when passing in APInt.

I briefly went through the uses of these matchers and didn't spot any places where allowing poison would be obviously wrong (I initially thought foldSelectICmpAndBinOp is problematic, but because it does not reuse the original constants, it's fine).

A problem here is that, despite these matchers appearing in a decent number of places, we have very little pre-existing test coverage for these folds with poison vectors, so we don't benefit from alive2 verification. It would be quite a bit of effort to add this coverage for all affected folds though. Thoughts on that?

This allows vector splats containing poison for matchers using api_pred_ty, making the behavior consistent with cst_pred_ty. In other words, previously `m_NonNegative()` allowed splats with poison, while `m_NonNegative(C)` did not. Now both allow them. The affected matchers are m_MaxSignedValue, m_Negative, m_NonNegative, m_StrictlyPositive, m_NonPositive, m_Power2, m_Power2OrZero, m_NegatedPower2, m_NegatedPower2OrZero, m_LowBitMask and m_LowBitMaskOrZero when passing in APInt. I briefly went through the uses of these matchers and didn't spot any places where allowing poison would be obviously wrong (I initially thought foldSelectICmpAndBinOp is problematic, but because it does not reuse the original constants, it's fine). A problem here is that, despite these matchers appearing in a decent number of places, we have very little pre-existing test coverage for these folds with poison vectors, so we don't benefit from alive2 verification. It would be quite a bit of effort to add this coverage for all affected folds though. Thoughts on that?

llvmbot · 2024-04-18T08:50:27Z

@llvm/pr-subscribers-llvm-transforms

Author: Nikita Popov (nikic)

Changes

This allows vector splats containing poison for matchers using api_pred_ty, making the behavior consistent with cst_pred_ty. In other words, previously m_NonNegative() allowed splats with poison, while m_NonNegative(C) did not. Now both allow them.

The affected matchers are m_MaxSignedValue, m_Negative, m_NonNegative, m_StrictlyPositive, m_NonPositive, m_Power2, m_Power2OrZero, m_NegatedPower2, m_NegatedPower2OrZero, m_LowBitMask and m_LowBitMaskOrZero when passing in APInt.

I briefly went through the uses of these matchers and didn't spot any places where allowing poison would be obviously wrong (I initially thought foldSelectICmpAndBinOp is problematic, but because it does not reuse the original constants, it's fine).

A problem here is that, despite these matchers appearing in a decent number of places, we have very little pre-existing test coverage for these folds with poison vectors, so we don't benefit from alive2 verification. It would be quite a bit of effort to add this coverage for all affected folds though. Thoughts on that?

Full diff: https://github.com/llvm/llvm-project/pull/89188.diff

3 Files Affected:

(modified) llvm/include/llvm/IR/PatternMatch.h (+2-1)
(modified) llvm/test/Transforms/InstCombine/select-with-bitwise-ops.ll (+42)
(modified) llvm/test/Transforms/InstCombine/signed-truncation-check.ll (+12-30)

diff --git a/llvm/include/llvm/IR/PatternMatch.h b/llvm/include/llvm/IR/PatternMatch.h
index 30e9c3900c8661..1b418a98545414 100644
--- a/llvm/include/llvm/IR/PatternMatch.h
+++ b/llvm/include/llvm/IR/PatternMatch.h
@@ -406,7 +406,8 @@ template <typename Predicate> struct api_pred_ty : public Predicate {
       }
     if (V->getType()->isVectorTy())
       if (const auto *C = dyn_cast<Constant>(V))
-        if (auto *CI = dyn_cast_or_null<ConstantInt>(C->getSplatValue()))
+        if (auto *CI = dyn_cast_or_null<ConstantInt>(
+                C->getSplatValue(/*AllowPoison=*/true)))
           if (this->isValue(CI->getValue())) {
             Res = &CI->getValue();
             return true;
diff --git a/llvm/test/Transforms/InstCombine/select-with-bitwise-ops.ll b/llvm/test/Transforms/InstCombine/select-with-bitwise-ops.ll
index 71d898a3ed9bd0..416a6d71055b62 100644
--- a/llvm/test/Transforms/InstCombine/select-with-bitwise-ops.ll
+++ b/llvm/test/Transforms/InstCombine/select-with-bitwise-ops.ll
@@ -34,6 +34,48 @@ define <2 x i32> @select_icmp_eq_and_1_0_or_2_vec(<2 x i32> %x, <2 x i32> %y) {
   ret <2 x i32> %select
 }
 
+define <2 x i32> @select_icmp_eq_and_1_0_or_2_vec_poison1(<2 x i32> %x, <2 x i32> %y) {
+; CHECK-LABEL: @select_icmp_eq_and_1_0_or_2_vec_poison1(
+; CHECK-NEXT:    [[AND:%.*]] = and <2 x i32> [[X:%.*]], <i32 1, i32 poison>
+; CHECK-NEXT:    [[TMP1:%.*]] = shl nuw nsw <2 x i32> [[AND]], <i32 1, i32 1>
+; CHECK-NEXT:    [[SELECT:%.*]] = or <2 x i32> [[TMP1]], [[Y:%.*]]
+; CHECK-NEXT:    ret <2 x i32> [[SELECT]]
+;
+  %and = and <2 x i32> %x, <i32 1, i32 poison>
+  %cmp = icmp eq <2 x i32> %and, zeroinitializer
+  %or = or <2 x i32> %y, <i32 2, i32 2>
+  %select = select <2 x i1> %cmp, <2 x i32> %y, <2 x i32> %or
+  ret <2 x i32> %select
+}
+
+define <2 x i32> @select_icmp_eq_and_1_0_or_2_vec_poison2(<2 x i32> %x, <2 x i32> %y) {
+; CHECK-LABEL: @select_icmp_eq_and_1_0_or_2_vec_poison2(
+; CHECK-NEXT:    [[AND:%.*]] = shl <2 x i32> [[X:%.*]], <i32 1, i32 1>
+; CHECK-NEXT:    [[TMP1:%.*]] = and <2 x i32> [[AND]], <i32 2, i32 2>
+; CHECK-NEXT:    [[SELECT:%.*]] = or <2 x i32> [[TMP1]], [[Y:%.*]]
+; CHECK-NEXT:    ret <2 x i32> [[SELECT]]
+;
+  %and = and <2 x i32> %x, <i32 1, i32 1>
+  %cmp = icmp eq <2 x i32> %and, <i32 0, i32 poison>
+  %or = or <2 x i32> %y, <i32 2, i32 2>
+  %select = select <2 x i1> %cmp, <2 x i32> %y, <2 x i32> %or
+  ret <2 x i32> %select
+}
+
+define <2 x i32> @select_icmp_eq_and_1_0_or_2_vec_poison3(<2 x i32> %x, <2 x i32> %y) {
+; CHECK-LABEL: @select_icmp_eq_and_1_0_or_2_vec_poison3(
+; CHECK-NEXT:    [[AND:%.*]] = shl <2 x i32> [[X:%.*]], <i32 1, i32 1>
+; CHECK-NEXT:    [[TMP1:%.*]] = and <2 x i32> [[AND]], <i32 2, i32 2>
+; CHECK-NEXT:    [[SELECT:%.*]] = or <2 x i32> [[TMP1]], [[Y:%.*]]
+; CHECK-NEXT:    ret <2 x i32> [[SELECT]]
+;
+  %and = and <2 x i32> %x, <i32 1, i32 1>
+  %cmp = icmp eq <2 x i32> %and, zeroinitializer
+  %or = or <2 x i32> %y, <i32 2, i32 poison>
+  %select = select <2 x i1> %cmp, <2 x i32> %y, <2 x i32> %or
+  ret <2 x i32> %select
+}
+
 define i32 @select_icmp_eq_and_1_0_xor_2(i32 %x, i32 %y) {
 ; CHECK-LABEL: @select_icmp_eq_and_1_0_xor_2(
 ; CHECK-NEXT:    [[AND:%.*]] = shl i32 [[X:%.*]], 1
diff --git a/llvm/test/Transforms/InstCombine/signed-truncation-check.ll b/llvm/test/Transforms/InstCombine/signed-truncation-check.ll
index 17cd536de91f4d..7e762627e5ec02 100644
--- a/llvm/test/Transforms/InstCombine/signed-truncation-check.ll
+++ b/llvm/test/Transforms/InstCombine/signed-truncation-check.ll
@@ -212,11 +212,8 @@ define <3 x i1> @positive_vec_poison0(<3 x i32> %arg) {
 
 define <3 x i1> @positive_vec_poison1(<3 x i32> %arg) {
 ; CHECK-LABEL: @positive_vec_poison1(
-; CHECK-NEXT:    [[T1:%.*]] = icmp sgt <3 x i32> [[ARG:%.*]], <i32 -1, i32 -1, i32 -1>
-; CHECK-NEXT:    [[T2:%.*]] = add <3 x i32> [[ARG]], <i32 128, i32 poison, i32 128>
-; CHECK-NEXT:    [[T3:%.*]] = icmp ult <3 x i32> [[T2]], <i32 256, i32 256, i32 256>
-; CHECK-NEXT:    [[T4:%.*]] = and <3 x i1> [[T1]], [[T3]]
-; CHECK-NEXT:    ret <3 x i1> [[T4]]
+; CHECK-NEXT:    [[T4_SIMPLIFIED:%.*]] = icmp ult <3 x i32> [[ARG:%.*]], <i32 128, i32 128, i32 128>
+; CHECK-NEXT:    ret <3 x i1> [[T4_SIMPLIFIED]]
 ;
   %t1 = icmp sgt <3 x i32> %arg, <i32 -1, i32 -1, i32 -1>
   %t2 = add <3 x i32> %arg, <i32 128, i32 poison, i32 128>
@@ -227,11 +224,8 @@ define <3 x i1> @positive_vec_poison1(<3 x i32> %arg) {
 
 define <3 x i1> @positive_vec_poison2(<3 x i32> %arg) {
 ; CHECK-LABEL: @positive_vec_poison2(
-; CHECK-NEXT:    [[T1:%.*]] = icmp sgt <3 x i32> [[ARG:%.*]], <i32 -1, i32 -1, i32 -1>
-; CHECK-NEXT:    [[T2:%.*]] = add <3 x i32> [[ARG]], <i32 128, i32 128, i32 128>
-; CHECK-NEXT:    [[T3:%.*]] = icmp ult <3 x i32> [[T2]], <i32 256, i32 poison, i32 256>
-; CHECK-NEXT:    [[T4:%.*]] = and <3 x i1> [[T1]], [[T3]]
-; CHECK-NEXT:    ret <3 x i1> [[T4]]
+; CHECK-NEXT:    [[T4_SIMPLIFIED:%.*]] = icmp ult <3 x i32> [[ARG:%.*]], <i32 128, i32 128, i32 128>
+; CHECK-NEXT:    ret <3 x i1> [[T4_SIMPLIFIED]]
 ;
   %t1 = icmp sgt <3 x i32> %arg, <i32 -1, i32 -1, i32 -1>
   %t2 = add <3 x i32> %arg, <i32 128, i32 128, i32 128>
@@ -242,11 +236,8 @@ define <3 x i1> @positive_vec_poison2(<3 x i32> %arg) {
 
 define <3 x i1> @positive_vec_poison3(<3 x i32> %arg) {
 ; CHECK-LABEL: @positive_vec_poison3(
-; CHECK-NEXT:    [[T1:%.*]] = icmp sgt <3 x i32> [[ARG:%.*]], <i32 -1, i32 poison, i32 -1>
-; CHECK-NEXT:    [[T2:%.*]] = add <3 x i32> [[ARG]], <i32 128, i32 poison, i32 128>
-; CHECK-NEXT:    [[T3:%.*]] = icmp ult <3 x i32> [[T2]], <i32 256, i32 256, i32 256>
-; CHECK-NEXT:    [[T4:%.*]] = and <3 x i1> [[T1]], [[T3]]
-; CHECK-NEXT:    ret <3 x i1> [[T4]]
+; CHECK-NEXT:    [[T4_SIMPLIFIED:%.*]] = icmp ult <3 x i32> [[ARG:%.*]], <i32 128, i32 128, i32 128>
+; CHECK-NEXT:    ret <3 x i1> [[T4_SIMPLIFIED]]
 ;
   %t1 = icmp sgt <3 x i32> %arg, <i32 -1, i32 poison, i32 -1>
   %t2 = add <3 x i32> %arg, <i32 128, i32 poison, i32 128>
@@ -257,11 +248,8 @@ define <3 x i1> @positive_vec_poison3(<3 x i32> %arg) {
 
 define <3 x i1> @positive_vec_poison4(<3 x i32> %arg) {
 ; CHECK-LABEL: @positive_vec_poison4(
-; CHECK-NEXT:    [[T1:%.*]] = icmp sgt <3 x i32> [[ARG:%.*]], <i32 -1, i32 poison, i32 -1>
-; CHECK-NEXT:    [[T2:%.*]] = add <3 x i32> [[ARG]], <i32 128, i32 128, i32 128>
-; CHECK-NEXT:    [[T3:%.*]] = icmp ult <3 x i32> [[T2]], <i32 256, i32 poison, i32 256>
-; CHECK-NEXT:    [[T4:%.*]] = and <3 x i1> [[T1]], [[T3]]
-; CHECK-NEXT:    ret <3 x i1> [[T4]]
+; CHECK-NEXT:    [[T4_SIMPLIFIED:%.*]] = icmp ult <3 x i32> [[ARG:%.*]], <i32 128, i32 128, i32 128>
+; CHECK-NEXT:    ret <3 x i1> [[T4_SIMPLIFIED]]
 ;
   %t1 = icmp sgt <3 x i32> %arg, <i32 -1, i32 poison, i32 -1>
   %t2 = add <3 x i32> %arg, <i32 128, i32 128, i32 128>
@@ -272,11 +260,8 @@ define <3 x i1> @positive_vec_poison4(<3 x i32> %arg) {
 
 define <3 x i1> @positive_vec_poison5(<3 x i32> %arg) {
 ; CHECK-LABEL: @positive_vec_poison5(
-; CHECK-NEXT:    [[T1:%.*]] = icmp sgt <3 x i32> [[ARG:%.*]], <i32 -1, i32 -1, i32 -1>
-; CHECK-NEXT:    [[T2:%.*]] = add <3 x i32> [[ARG]], <i32 128, i32 poison, i32 128>
-; CHECK-NEXT:    [[T3:%.*]] = icmp ult <3 x i32> [[T2]], <i32 256, i32 poison, i32 256>
-; CHECK-NEXT:    [[T4:%.*]] = and <3 x i1> [[T1]], [[T3]]
-; CHECK-NEXT:    ret <3 x i1> [[T4]]
+; CHECK-NEXT:    [[T4_SIMPLIFIED:%.*]] = icmp ult <3 x i32> [[ARG:%.*]], <i32 128, i32 128, i32 128>
+; CHECK-NEXT:    ret <3 x i1> [[T4_SIMPLIFIED]]
 ;
   %t1 = icmp sgt <3 x i32> %arg, <i32 -1, i32 -1, i32 -1>
   %t2 = add <3 x i32> %arg, <i32 128, i32 poison, i32 128>
@@ -287,11 +272,8 @@ define <3 x i1> @positive_vec_poison5(<3 x i32> %arg) {
 
 define <3 x i1> @positive_vec_poison6(<3 x i32> %arg) {
 ; CHECK-LABEL: @positive_vec_poison6(
-; CHECK-NEXT:    [[T1:%.*]] = icmp sgt <3 x i32> [[ARG:%.*]], <i32 -1, i32 poison, i32 -1>
-; CHECK-NEXT:    [[T2:%.*]] = add <3 x i32> [[ARG]], <i32 128, i32 poison, i32 128>
-; CHECK-NEXT:    [[T3:%.*]] = icmp ult <3 x i32> [[T2]], <i32 256, i32 poison, i32 256>
-; CHECK-NEXT:    [[T4:%.*]] = and <3 x i1> [[T1]], [[T3]]
-; CHECK-NEXT:    ret <3 x i1> [[T4]]
+; CHECK-NEXT:    [[T4_SIMPLIFIED:%.*]] = icmp ult <3 x i32> [[ARG:%.*]], <i32 128, i32 128, i32 128>
+; CHECK-NEXT:    ret <3 x i1> [[T4_SIMPLIFIED]]
 ;
   %t1 = icmp sgt <3 x i32> %arg, <i32 -1, i32 poison, i32 -1>
   %t2 = add <3 x i32> %arg, <i32 128, i32 poison, i32 128>

llvmbot · 2024-04-18T08:50:27Z

@llvm/pr-subscribers-llvm-ir

Author: Nikita Popov (nikic)

Changes

This allows vector splats containing poison for matchers using api_pred_ty, making the behavior consistent with cst_pred_ty. In other words, previously m_NonNegative() allowed splats with poison, while m_NonNegative(C) did not. Now both allow them.

The affected matchers are m_MaxSignedValue, m_Negative, m_NonNegative, m_StrictlyPositive, m_NonPositive, m_Power2, m_Power2OrZero, m_NegatedPower2, m_NegatedPower2OrZero, m_LowBitMask and m_LowBitMaskOrZero when passing in APInt.

I briefly went through the uses of these matchers and didn't spot any places where allowing poison would be obviously wrong (I initially thought foldSelectICmpAndBinOp is problematic, but because it does not reuse the original constants, it's fine).

A problem here is that, despite these matchers appearing in a decent number of places, we have very little pre-existing test coverage for these folds with poison vectors, so we don't benefit from alive2 verification. It would be quite a bit of effort to add this coverage for all affected folds though. Thoughts on that?

Full diff: https://github.com/llvm/llvm-project/pull/89188.diff

3 Files Affected:

(modified) llvm/include/llvm/IR/PatternMatch.h (+2-1)
(modified) llvm/test/Transforms/InstCombine/select-with-bitwise-ops.ll (+42)
(modified) llvm/test/Transforms/InstCombine/signed-truncation-check.ll (+12-30)

diff --git a/llvm/include/llvm/IR/PatternMatch.h b/llvm/include/llvm/IR/PatternMatch.h
index 30e9c3900c8661..1b418a98545414 100644
--- a/llvm/include/llvm/IR/PatternMatch.h
+++ b/llvm/include/llvm/IR/PatternMatch.h
@@ -406,7 +406,8 @@ template <typename Predicate> struct api_pred_ty : public Predicate {
       }
     if (V->getType()->isVectorTy())
       if (const auto *C = dyn_cast<Constant>(V))
-        if (auto *CI = dyn_cast_or_null<ConstantInt>(C->getSplatValue()))
+        if (auto *CI = dyn_cast_or_null<ConstantInt>(
+                C->getSplatValue(/*AllowPoison=*/true)))
           if (this->isValue(CI->getValue())) {
             Res = &CI->getValue();
             return true;
diff --git a/llvm/test/Transforms/InstCombine/select-with-bitwise-ops.ll b/llvm/test/Transforms/InstCombine/select-with-bitwise-ops.ll
index 71d898a3ed9bd0..416a6d71055b62 100644
--- a/llvm/test/Transforms/InstCombine/select-with-bitwise-ops.ll
+++ b/llvm/test/Transforms/InstCombine/select-with-bitwise-ops.ll
@@ -34,6 +34,48 @@ define <2 x i32> @select_icmp_eq_and_1_0_or_2_vec(<2 x i32> %x, <2 x i32> %y) {
   ret <2 x i32> %select
 }
 
+define <2 x i32> @select_icmp_eq_and_1_0_or_2_vec_poison1(<2 x i32> %x, <2 x i32> %y) {
+; CHECK-LABEL: @select_icmp_eq_and_1_0_or_2_vec_poison1(
+; CHECK-NEXT:    [[AND:%.*]] = and <2 x i32> [[X:%.*]], <i32 1, i32 poison>
+; CHECK-NEXT:    [[TMP1:%.*]] = shl nuw nsw <2 x i32> [[AND]], <i32 1, i32 1>
+; CHECK-NEXT:    [[SELECT:%.*]] = or <2 x i32> [[TMP1]], [[Y:%.*]]
+; CHECK-NEXT:    ret <2 x i32> [[SELECT]]
+;
+  %and = and <2 x i32> %x, <i32 1, i32 poison>
+  %cmp = icmp eq <2 x i32> %and, zeroinitializer
+  %or = or <2 x i32> %y, <i32 2, i32 2>
+  %select = select <2 x i1> %cmp, <2 x i32> %y, <2 x i32> %or
+  ret <2 x i32> %select
+}
+
+define <2 x i32> @select_icmp_eq_and_1_0_or_2_vec_poison2(<2 x i32> %x, <2 x i32> %y) {
+; CHECK-LABEL: @select_icmp_eq_and_1_0_or_2_vec_poison2(
+; CHECK-NEXT:    [[AND:%.*]] = shl <2 x i32> [[X:%.*]], <i32 1, i32 1>
+; CHECK-NEXT:    [[TMP1:%.*]] = and <2 x i32> [[AND]], <i32 2, i32 2>
+; CHECK-NEXT:    [[SELECT:%.*]] = or <2 x i32> [[TMP1]], [[Y:%.*]]
+; CHECK-NEXT:    ret <2 x i32> [[SELECT]]
+;
+  %and = and <2 x i32> %x, <i32 1, i32 1>
+  %cmp = icmp eq <2 x i32> %and, <i32 0, i32 poison>
+  %or = or <2 x i32> %y, <i32 2, i32 2>
+  %select = select <2 x i1> %cmp, <2 x i32> %y, <2 x i32> %or
+  ret <2 x i32> %select
+}
+
+define <2 x i32> @select_icmp_eq_and_1_0_or_2_vec_poison3(<2 x i32> %x, <2 x i32> %y) {
+; CHECK-LABEL: @select_icmp_eq_and_1_0_or_2_vec_poison3(
+; CHECK-NEXT:    [[AND:%.*]] = shl <2 x i32> [[X:%.*]], <i32 1, i32 1>
+; CHECK-NEXT:    [[TMP1:%.*]] = and <2 x i32> [[AND]], <i32 2, i32 2>
+; CHECK-NEXT:    [[SELECT:%.*]] = or <2 x i32> [[TMP1]], [[Y:%.*]]
+; CHECK-NEXT:    ret <2 x i32> [[SELECT]]
+;
+  %and = and <2 x i32> %x, <i32 1, i32 1>
+  %cmp = icmp eq <2 x i32> %and, zeroinitializer
+  %or = or <2 x i32> %y, <i32 2, i32 poison>
+  %select = select <2 x i1> %cmp, <2 x i32> %y, <2 x i32> %or
+  ret <2 x i32> %select
+}
+
 define i32 @select_icmp_eq_and_1_0_xor_2(i32 %x, i32 %y) {
 ; CHECK-LABEL: @select_icmp_eq_and_1_0_xor_2(
 ; CHECK-NEXT:    [[AND:%.*]] = shl i32 [[X:%.*]], 1
diff --git a/llvm/test/Transforms/InstCombine/signed-truncation-check.ll b/llvm/test/Transforms/InstCombine/signed-truncation-check.ll
index 17cd536de91f4d..7e762627e5ec02 100644
--- a/llvm/test/Transforms/InstCombine/signed-truncation-check.ll
+++ b/llvm/test/Transforms/InstCombine/signed-truncation-check.ll
@@ -212,11 +212,8 @@ define <3 x i1> @positive_vec_poison0(<3 x i32> %arg) {
 
 define <3 x i1> @positive_vec_poison1(<3 x i32> %arg) {
 ; CHECK-LABEL: @positive_vec_poison1(
-; CHECK-NEXT:    [[T1:%.*]] = icmp sgt <3 x i32> [[ARG:%.*]], <i32 -1, i32 -1, i32 -1>
-; CHECK-NEXT:    [[T2:%.*]] = add <3 x i32> [[ARG]], <i32 128, i32 poison, i32 128>
-; CHECK-NEXT:    [[T3:%.*]] = icmp ult <3 x i32> [[T2]], <i32 256, i32 256, i32 256>
-; CHECK-NEXT:    [[T4:%.*]] = and <3 x i1> [[T1]], [[T3]]
-; CHECK-NEXT:    ret <3 x i1> [[T4]]
+; CHECK-NEXT:    [[T4_SIMPLIFIED:%.*]] = icmp ult <3 x i32> [[ARG:%.*]], <i32 128, i32 128, i32 128>
+; CHECK-NEXT:    ret <3 x i1> [[T4_SIMPLIFIED]]
 ;
   %t1 = icmp sgt <3 x i32> %arg, <i32 -1, i32 -1, i32 -1>
   %t2 = add <3 x i32> %arg, <i32 128, i32 poison, i32 128>
@@ -227,11 +224,8 @@ define <3 x i1> @positive_vec_poison1(<3 x i32> %arg) {
 
 define <3 x i1> @positive_vec_poison2(<3 x i32> %arg) {
 ; CHECK-LABEL: @positive_vec_poison2(
-; CHECK-NEXT:    [[T1:%.*]] = icmp sgt <3 x i32> [[ARG:%.*]], <i32 -1, i32 -1, i32 -1>
-; CHECK-NEXT:    [[T2:%.*]] = add <3 x i32> [[ARG]], <i32 128, i32 128, i32 128>
-; CHECK-NEXT:    [[T3:%.*]] = icmp ult <3 x i32> [[T2]], <i32 256, i32 poison, i32 256>
-; CHECK-NEXT:    [[T4:%.*]] = and <3 x i1> [[T1]], [[T3]]
-; CHECK-NEXT:    ret <3 x i1> [[T4]]
+; CHECK-NEXT:    [[T4_SIMPLIFIED:%.*]] = icmp ult <3 x i32> [[ARG:%.*]], <i32 128, i32 128, i32 128>
+; CHECK-NEXT:    ret <3 x i1> [[T4_SIMPLIFIED]]
 ;
   %t1 = icmp sgt <3 x i32> %arg, <i32 -1, i32 -1, i32 -1>
   %t2 = add <3 x i32> %arg, <i32 128, i32 128, i32 128>
@@ -242,11 +236,8 @@ define <3 x i1> @positive_vec_poison2(<3 x i32> %arg) {
 
 define <3 x i1> @positive_vec_poison3(<3 x i32> %arg) {
 ; CHECK-LABEL: @positive_vec_poison3(
-; CHECK-NEXT:    [[T1:%.*]] = icmp sgt <3 x i32> [[ARG:%.*]], <i32 -1, i32 poison, i32 -1>
-; CHECK-NEXT:    [[T2:%.*]] = add <3 x i32> [[ARG]], <i32 128, i32 poison, i32 128>
-; CHECK-NEXT:    [[T3:%.*]] = icmp ult <3 x i32> [[T2]], <i32 256, i32 256, i32 256>
-; CHECK-NEXT:    [[T4:%.*]] = and <3 x i1> [[T1]], [[T3]]
-; CHECK-NEXT:    ret <3 x i1> [[T4]]
+; CHECK-NEXT:    [[T4_SIMPLIFIED:%.*]] = icmp ult <3 x i32> [[ARG:%.*]], <i32 128, i32 128, i32 128>
+; CHECK-NEXT:    ret <3 x i1> [[T4_SIMPLIFIED]]
 ;
   %t1 = icmp sgt <3 x i32> %arg, <i32 -1, i32 poison, i32 -1>
   %t2 = add <3 x i32> %arg, <i32 128, i32 poison, i32 128>
@@ -257,11 +248,8 @@ define <3 x i1> @positive_vec_poison3(<3 x i32> %arg) {
 
 define <3 x i1> @positive_vec_poison4(<3 x i32> %arg) {
 ; CHECK-LABEL: @positive_vec_poison4(
-; CHECK-NEXT:    [[T1:%.*]] = icmp sgt <3 x i32> [[ARG:%.*]], <i32 -1, i32 poison, i32 -1>
-; CHECK-NEXT:    [[T2:%.*]] = add <3 x i32> [[ARG]], <i32 128, i32 128, i32 128>
-; CHECK-NEXT:    [[T3:%.*]] = icmp ult <3 x i32> [[T2]], <i32 256, i32 poison, i32 256>
-; CHECK-NEXT:    [[T4:%.*]] = and <3 x i1> [[T1]], [[T3]]
-; CHECK-NEXT:    ret <3 x i1> [[T4]]
+; CHECK-NEXT:    [[T4_SIMPLIFIED:%.*]] = icmp ult <3 x i32> [[ARG:%.*]], <i32 128, i32 128, i32 128>
+; CHECK-NEXT:    ret <3 x i1> [[T4_SIMPLIFIED]]
 ;
   %t1 = icmp sgt <3 x i32> %arg, <i32 -1, i32 poison, i32 -1>
   %t2 = add <3 x i32> %arg, <i32 128, i32 128, i32 128>
@@ -272,11 +260,8 @@ define <3 x i1> @positive_vec_poison4(<3 x i32> %arg) {
 
 define <3 x i1> @positive_vec_poison5(<3 x i32> %arg) {
 ; CHECK-LABEL: @positive_vec_poison5(
-; CHECK-NEXT:    [[T1:%.*]] = icmp sgt <3 x i32> [[ARG:%.*]], <i32 -1, i32 -1, i32 -1>
-; CHECK-NEXT:    [[T2:%.*]] = add <3 x i32> [[ARG]], <i32 128, i32 poison, i32 128>
-; CHECK-NEXT:    [[T3:%.*]] = icmp ult <3 x i32> [[T2]], <i32 256, i32 poison, i32 256>
-; CHECK-NEXT:    [[T4:%.*]] = and <3 x i1> [[T1]], [[T3]]
-; CHECK-NEXT:    ret <3 x i1> [[T4]]
+; CHECK-NEXT:    [[T4_SIMPLIFIED:%.*]] = icmp ult <3 x i32> [[ARG:%.*]], <i32 128, i32 128, i32 128>
+; CHECK-NEXT:    ret <3 x i1> [[T4_SIMPLIFIED]]
 ;
   %t1 = icmp sgt <3 x i32> %arg, <i32 -1, i32 -1, i32 -1>
   %t2 = add <3 x i32> %arg, <i32 128, i32 poison, i32 128>
@@ -287,11 +272,8 @@ define <3 x i1> @positive_vec_poison5(<3 x i32> %arg) {
 
 define <3 x i1> @positive_vec_poison6(<3 x i32> %arg) {
 ; CHECK-LABEL: @positive_vec_poison6(
-; CHECK-NEXT:    [[T1:%.*]] = icmp sgt <3 x i32> [[ARG:%.*]], <i32 -1, i32 poison, i32 -1>
-; CHECK-NEXT:    [[T2:%.*]] = add <3 x i32> [[ARG]], <i32 128, i32 poison, i32 128>
-; CHECK-NEXT:    [[T3:%.*]] = icmp ult <3 x i32> [[T2]], <i32 256, i32 poison, i32 256>
-; CHECK-NEXT:    [[T4:%.*]] = and <3 x i1> [[T1]], [[T3]]
-; CHECK-NEXT:    ret <3 x i1> [[T4]]
+; CHECK-NEXT:    [[T4_SIMPLIFIED:%.*]] = icmp ult <3 x i32> [[ARG:%.*]], <i32 128, i32 128, i32 128>
+; CHECK-NEXT:    ret <3 x i1> [[T4_SIMPLIFIED]]
 ;
   %t1 = icmp sgt <3 x i32> %arg, <i32 -1, i32 poison, i32 -1>
   %t2 = add <3 x i32> %arg, <i32 128, i32 poison, i32 128>

nunoplopes · 2024-04-18T08:53:51Z

maybe @regehr can help with fuzzing vector instructions.

dtcxzyw

LGTM.

dtcxzyw · 2024-04-18T13:31:16Z

A problem here is that, despite these matchers appearing in a decent number of places, we have very little pre-existing test coverage for these folds with poison vectors, so we don't benefit from alive2 verification. It would be quite a bit of effort to add this coverage for all affected folds though. Thoughts on that?

I have a "vectorizer" for RVV codegen testing. It converts a scalar function into a vectorized version. Do you think it is helpful? If so, I am willing to adjust it to generate vector tests from existing scalar tests :)

See also: https://github.com/dtcxzyw/llvm-codegen-benchmark/blob/main/vectorize.cpp

regehr · 2024-04-18T18:43:43Z

maybe @regehr can help with fuzzing vector instructions.

yes, we have a mutation-based fuzzer for IR. currently I'm using it on my biggest machine to look for AArch64 backend bugs, but when this run finishes, I can turn the tools back towards middle-end bugs. it's easy to filter out inputs that don't contain any vectors. I'll do that next.

nikic · 2024-04-19T00:11:36Z

A problem here is that, despite these matchers appearing in a decent number of places, we have very little pre-existing test coverage for these folds with poison vectors, so we don't benefit from alive2 verification. It would be quite a bit of effort to add this coverage for all affected folds though. Thoughts on that?

I have a "vectorizer" for RVV codegen testing. It converts a scalar function into a vectorized version. Do you think it is helpful? If so, I am willing to adjust it to generate vector tests from existing scalar tests :)

See also: https://github.com/dtcxzyw/llvm-codegen-benchmark/blob/main/vectorize.cpp

This looks like a nice idea. If we can "vectorize" all the InstCombine test, replace one element in each constant with poison and run alive2 over it, we could be pretty confident that handling poison in more cases does not introduce issues.

nikic requested review from nunoplopes, dtcxzyw and goldsteinn April 18, 2024 08:49

llvmbot added llvm:ir llvm:transforms labels Apr 18, 2024

nikic mentioned this pull request Apr 18, 2024

[IR][PatternMatch] Only accept poison in getSplatValue() #89159

Merged

dtcxzyw approved these changes Apr 18, 2024

View reviewed changes

nikic merged commit 396cdab into llvm:main Apr 19, 2024
6 of 7 checks passed

nikic deleted the api-pred-allow-poison branch April 19, 2024 00:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PatternMatch] Allow poison in api_pred_ty matchers #89188

[PatternMatch] Allow poison in api_pred_ty matchers #89188

nikic commented Apr 18, 2024

llvmbot commented Apr 18, 2024

llvmbot commented Apr 18, 2024

nunoplopes commented Apr 18, 2024

dtcxzyw left a comment

dtcxzyw commented Apr 18, 2024

regehr commented Apr 18, 2024

nikic commented Apr 19, 2024

[PatternMatch] Allow poison in api_pred_ty matchers #89188

[PatternMatch] Allow poison in api_pred_ty matchers #89188

Conversation

nikic commented Apr 18, 2024

llvmbot commented Apr 18, 2024

llvmbot commented Apr 18, 2024

nunoplopes commented Apr 18, 2024

dtcxzyw left a comment

Choose a reason for hiding this comment

dtcxzyw commented Apr 18, 2024

regehr commented Apr 18, 2024

nikic commented Apr 19, 2024