Skip to content

Commit f2d555e

Browse files
nikicnikic
committed
[PatternMatch] Do not accept undef elements in m_AllOnes() and friends
Change all the cstval_pred_ty based PatternMatch helpers (things like m_AllOnes and m_Zero) to only allow poison elements inside vector splats, not undef elements. Historically, we used to represent non-demanded elements in vectors using undef. Nowadays, we use poison instead. As such, I believe that support for undef in vector splats is no longer useful. At the same time, while poison splat elements are pretty much always safe to ignore, this is not generally the case for undef elements. We have existing miscompiles in our tests due to this (see the masked-merge-*.ll tests changed here) and it's easy to miss such cases in the future, now that we write tests using poison instead of undef elements. I think overall, keeping support for undef elements no longer makes sense, and we should drop it. Once this is done consistently, I think we may also consider allowing poison in m_APInt by default, as doing that change is much less risky than doing the same with undef. This PR is a stub for discussion -- this change has more than a hundred test failures in InstCombine alone, and I don't want to update all of them before we reach a consensus. The abs-1.ll test is a representative example of how I would update most tests: By replacing undef with poison. I don't think there is value in retaining the undef test coverage anymore at this point.
1 parent e8a3b72 commit f2d555e

File tree

5 files changed

+61
-16
lines changed

5 files changed

+61
-16
lines changed

‎llvm/include/llvm/IR/PatternMatch.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ template <int64_t Val> inline constantint_match<Val> m_ConstantInt() {
345345

346346
/// This helper class is used to match constant scalars, vector splats,
347347
/// and fixed width vectors that satisfy a specified predicate.
348-
/// For fixed width vector constants, undefined elements are ignored.
348+
/// For fixed width vector constants, poison elements are ignored.
349349
template <typename Predicate, typename ConstantVal>
350350
struct cstval_pred_ty : public Predicate {
351351
template <typename ITy> bool match(ITy *V) {
@@ -364,19 +364,19 @@ struct cstval_pred_ty : public Predicate {
364364
// Non-splat vector constant: check each element for a match.
365365
unsigned NumElts = FVTy->getNumElements();
366366
assert(NumElts != 0 && "Constant vector with no elements?");
367-
bool HasNonUndefElements = false;
367+
bool HasNonPoisonElements = false;
368368
for (unsigned i = 0; i != NumElts; ++i) {
369369
Constant *Elt = C->getAggregateElement(i);
370370
if (!Elt)
371371
return false;
372-
if (isa<UndefValue>(Elt))
372+
if (isa<PoisonValue>(Elt))
373373
continue;
374374
auto *CV = dyn_cast<ConstantVal>(Elt);
375375
if (!CV || !this->isValue(CV->getValue()))
376376
return false;
377-
HasNonUndefElements = true;
377+
HasNonPoisonElements = true;
378378
}
379-
return HasNonUndefElements;
379+
return HasNonPoisonElements;
380380
}
381381
}
382382
return false;

‎llvm/test/Transforms/InstCombine/abs-1.ll

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,14 @@ define <2 x i8> @abs_canonical_2(<2 x i8> %x) {
6363
ret <2 x i8> %abs
6464
}
6565

66-
; Even if a constant has undef elements.
66+
; Even if a constant has poison elements.
6767

68-
define <2 x i8> @abs_canonical_2_vec_undef_elts(<2 x i8> %x) {
69-
; CHECK-LABEL: @abs_canonical_2_vec_undef_elts(
68+
define <2 x i8> @abs_canonical_2_vec_poison_elts(<2 x i8> %x) {
69+
; CHECK-LABEL: @abs_canonical_2_vec_poison_elts(
7070
; CHECK-NEXT: [[ABS:%.*]] = call <2 x i8> @llvm.abs.v2i8(<2 x i8> [[X:%.*]], i1 false)
7171
; CHECK-NEXT: ret <2 x i8> [[ABS]]
7272
;
73-
%cmp = icmp sgt <2 x i8> %x, <i8 undef, i8 -1>
73+
%cmp = icmp sgt <2 x i8> %x, <i8 poison, i8 -1>
7474
%neg = sub <2 x i8> zeroinitializer, %x
7575
%abs = select <2 x i1> %cmp, <2 x i8> %x, <2 x i8> %neg
7676
ret <2 x i8> %abs
@@ -208,15 +208,15 @@ define <2 x i8> @nabs_canonical_2(<2 x i8> %x) {
208208
ret <2 x i8> %abs
209209
}
210210

211-
; Even if a constant has undef elements.
211+
; Even if a constant has poison elements.
212212

213-
define <2 x i8> @nabs_canonical_2_vec_undef_elts(<2 x i8> %x) {
214-
; CHECK-LABEL: @nabs_canonical_2_vec_undef_elts(
213+
define <2 x i8> @nabs_canonical_2_vec_poison_elts(<2 x i8> %x) {
214+
; CHECK-LABEL: @nabs_canonical_2_vec_poison_elts(
215215
; CHECK-NEXT: [[TMP1:%.*]] = call <2 x i8> @llvm.abs.v2i8(<2 x i8> [[X:%.*]], i1 false)
216216
; CHECK-NEXT: [[ABS:%.*]] = sub <2 x i8> zeroinitializer, [[TMP1]]
217217
; CHECK-NEXT: ret <2 x i8> [[ABS]]
218218
;
219-
%cmp = icmp sgt <2 x i8> %x, <i8 -1, i8 undef>
219+
%cmp = icmp sgt <2 x i8> %x, <i8 -1, i8 poison>
220220
%neg = sub <2 x i8> zeroinitializer, %x
221221
%abs = select <2 x i1> %cmp, <2 x i8> %neg, <2 x i8> %x
222222
ret <2 x i8> %abs

‎llvm/test/Transforms/InstCombine/masked-merge-add.ll

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ define <3 x i32> @p_vec_undef(<3 x i32> %x, <3 x i32> %y, <3 x i32> noundef %m)
5151
; CHECK-NEXT: [[AND:%.*]] = and <3 x i32> [[X:%.*]], [[M:%.*]]
5252
; CHECK-NEXT: [[NEG:%.*]] = xor <3 x i32> [[M]], <i32 -1, i32 undef, i32 -1>
5353
; CHECK-NEXT: [[AND1:%.*]] = and <3 x i32> [[NEG]], [[Y:%.*]]
54-
; CHECK-NEXT: [[RET:%.*]] = or disjoint <3 x i32> [[AND]], [[AND1]]
54+
; CHECK-NEXT: [[RET:%.*]] = add <3 x i32> [[AND]], [[AND1]]
5555
; CHECK-NEXT: ret <3 x i32> [[RET]]
5656
;
5757
%and = and <3 x i32> %x, %m
@@ -61,6 +61,21 @@ define <3 x i32> @p_vec_undef(<3 x i32> %x, <3 x i32> %y, <3 x i32> noundef %m)
6161
ret <3 x i32> %ret
6262
}
6363

64+
define <3 x i32> @p_vec_poison(<3 x i32> %x, <3 x i32> %y, <3 x i32> noundef %m) {
65+
; CHECK-LABEL: @p_vec_poison(
66+
; CHECK-NEXT: [[AND:%.*]] = and <3 x i32> [[X:%.*]], [[M:%.*]]
67+
; CHECK-NEXT: [[NEG:%.*]] = xor <3 x i32> [[M]], <i32 -1, i32 poison, i32 -1>
68+
; CHECK-NEXT: [[AND1:%.*]] = and <3 x i32> [[NEG]], [[Y:%.*]]
69+
; CHECK-NEXT: [[RET:%.*]] = or disjoint <3 x i32> [[AND]], [[AND1]]
70+
; CHECK-NEXT: ret <3 x i32> [[RET]]
71+
;
72+
%and = and <3 x i32> %x, %m
73+
%neg = xor <3 x i32> %m, <i32 -1, i32 poison, i32 -1>
74+
%and1 = and <3 x i32> %neg, %y
75+
%ret = add <3 x i32> %and, %and1
76+
ret <3 x i32> %ret
77+
}
78+
6479
; ============================================================================ ;
6580
; Constant mask.
6681
; ============================================================================ ;

‎llvm/test/Transforms/InstCombine/masked-merge-or.ll

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ define <3 x i32> @p_vec_undef(<3 x i32> %x, <3 x i32> %y, <3 x i32> noundef %m)
5151
; CHECK-NEXT: [[AND:%.*]] = and <3 x i32> [[X:%.*]], [[M:%.*]]
5252
; CHECK-NEXT: [[NEG:%.*]] = xor <3 x i32> [[M]], <i32 -1, i32 undef, i32 -1>
5353
; CHECK-NEXT: [[AND1:%.*]] = and <3 x i32> [[NEG]], [[Y:%.*]]
54-
; CHECK-NEXT: [[RET:%.*]] = or disjoint <3 x i32> [[AND]], [[AND1]]
54+
; CHECK-NEXT: [[RET:%.*]] = or <3 x i32> [[AND]], [[AND1]]
5555
; CHECK-NEXT: ret <3 x i32> [[RET]]
5656
;
5757
%and = and <3 x i32> %x, %m
@@ -61,6 +61,21 @@ define <3 x i32> @p_vec_undef(<3 x i32> %x, <3 x i32> %y, <3 x i32> noundef %m)
6161
ret <3 x i32> %ret
6262
}
6363

64+
define <3 x i32> @p_vec_poison(<3 x i32> %x, <3 x i32> %y, <3 x i32> noundef %m) {
65+
; CHECK-LABEL: @p_vec_poison(
66+
; CHECK-NEXT: [[AND:%.*]] = and <3 x i32> [[X:%.*]], [[M:%.*]]
67+
; CHECK-NEXT: [[NEG:%.*]] = xor <3 x i32> [[M]], <i32 -1, i32 poison, i32 -1>
68+
; CHECK-NEXT: [[AND1:%.*]] = and <3 x i32> [[NEG]], [[Y:%.*]]
69+
; CHECK-NEXT: [[RET:%.*]] = or disjoint <3 x i32> [[AND]], [[AND1]]
70+
; CHECK-NEXT: ret <3 x i32> [[RET]]
71+
;
72+
%and = and <3 x i32> %x, %m
73+
%neg = xor <3 x i32> %m, <i32 -1, i32 poison, i32 -1>
74+
%and1 = and <3 x i32> %neg, %y
75+
%ret = or <3 x i32> %and, %and1
76+
ret <3 x i32> %ret
77+
}
78+
6479
; ============================================================================ ;
6580
; Constant mask.
6681
; ============================================================================ ;

‎llvm/test/Transforms/InstCombine/masked-merge-xor.ll

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ define <3 x i32> @p_vec_undef(<3 x i32> %x, <3 x i32> %y, <3 x i32> noundef %m)
5151
; CHECK-NEXT: [[AND:%.*]] = and <3 x i32> [[X:%.*]], [[M:%.*]]
5252
; CHECK-NEXT: [[NEG:%.*]] = xor <3 x i32> [[M]], <i32 -1, i32 undef, i32 -1>
5353
; CHECK-NEXT: [[AND1:%.*]] = and <3 x i32> [[NEG]], [[Y:%.*]]
54-
; CHECK-NEXT: [[RET:%.*]] = or disjoint <3 x i32> [[AND]], [[AND1]]
54+
; CHECK-NEXT: [[RET:%.*]] = xor <3 x i32> [[AND]], [[AND1]]
5555
; CHECK-NEXT: ret <3 x i32> [[RET]]
5656
;
5757
%and = and <3 x i32> %x, %m
@@ -61,6 +61,21 @@ define <3 x i32> @p_vec_undef(<3 x i32> %x, <3 x i32> %y, <3 x i32> noundef %m)
6161
ret <3 x i32> %ret
6262
}
6363

64+
define <3 x i32> @p_vec_poison(<3 x i32> %x, <3 x i32> %y, <3 x i32> noundef %m) {
65+
; CHECK-LABEL: @p_vec_poison(
66+
; CHECK-NEXT: [[AND:%.*]] = and <3 x i32> [[X:%.*]], [[M:%.*]]
67+
; CHECK-NEXT: [[NEG:%.*]] = xor <3 x i32> [[M]], <i32 -1, i32 poison, i32 -1>
68+
; CHECK-NEXT: [[AND1:%.*]] = and <3 x i32> [[NEG]], [[Y:%.*]]
69+
; CHECK-NEXT: [[RET:%.*]] = or disjoint <3 x i32> [[AND]], [[AND1]]
70+
; CHECK-NEXT: ret <3 x i32> [[RET]]
71+
;
72+
%and = and <3 x i32> %x, %m
73+
%neg = xor <3 x i32> %m, <i32 -1, i32 poison, i32 -1>
74+
%and1 = and <3 x i32> %neg, %y
75+
%ret = xor <3 x i32> %and, %and1
76+
ret <3 x i32> %ret
77+
}
78+
6479
; ============================================================================ ;
6580
; Constant mask.
6681
; ============================================================================ ;

0 commit comments

Comments
 (0)