Skip to content

Commit

Permalink
[PatternMatch, InstSimplify] enhance m_AllOnes() to ignore undef elem…
Browse files Browse the repository at this point in the history
…ents in vectors

Loosening the matcher definition reveals a subtle bug in InstSimplify (we should not
assume that because an operand constant matches that it's safe to return it as a result).

So I'm making that change here too (that diff could be independent, but I'm not sure how 
to reveal it before the matcher change).

This also seems like a good reason to *not* include matchers that capture the value.
We don't want to encourage the potential misstep of propagating undef values when it's
not allowed/intended.

I didn't include the capture variant option here or in the related rL325437 (m_One), 
but it already exists for other constant matchers.



git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@325466 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
rotateright committed Feb 18, 2018
1 parent 142011a commit 5563781
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 26 deletions.
19 changes: 8 additions & 11 deletions include/llvm/IR/PatternMatch.h
Original file line number Diff line number Diff line change
Expand Up @@ -182,17 +182,6 @@ struct match_nan {
/// Match an arbitrary NaN constant. This includes quiet and signalling nans.
inline match_nan m_NaN() { return match_nan(); }

struct match_all_ones {
template <typename ITy> bool match(ITy *V) {
if (const auto *C = dyn_cast<Constant>(V))
return C->isAllOnesValue();
return false;
}
};

/// Match an integer or vector with all bits set to true.
inline match_all_ones m_AllOnes() { return match_all_ones(); }

struct match_sign_mask {
template <typename ITy> bool match(ITy *V) {
if (const auto *C = dyn_cast<Constant>(V))
Expand Down Expand Up @@ -337,6 +326,14 @@ template <typename Predicate> struct api_pred_ty : public Predicate {
//
///////////////////////////////////////////////////////////////////////////////

struct is_all_ones {
bool isValue(const APInt &C) { return C.isAllOnesValue(); }
};
/// Match an integer or vector with all bits set.
inline cst_pred_ty<is_all_ones> m_AllOnes() {
return cst_pred_ty<is_all_ones>();
}

struct is_maxsignedvalue {
bool isValue(const APInt &C) { return C.isMaxSignedValue(); }
};
Expand Down
18 changes: 7 additions & 11 deletions lib/Analysis/InstructionSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1264,9 +1264,10 @@ static Value *SimplifyAShrInst(Value *Op0, Value *Op1, bool isExact,
MaxRecurse))
return V;

// all ones >>a X -> all ones
// all ones >>a X -> -1
// Do not return Op0 because it may contain undef elements if it's a vector.
if (match(Op0, m_AllOnes()))
return Op0;
return Constant::getAllOnesValue(Op0->getType());

// (X << A) >> A -> X
Value *X;
Expand Down Expand Up @@ -1783,21 +1784,16 @@ static Value *SimplifyOrInst(Value *Op0, Value *Op1, const SimplifyQuery &Q,
return C;

// X | undef -> -1
if (match(Op1, m_Undef()))
// X | -1 = -1
// Do not return Op1 because it may contain undef elements if it's a vector.
if (match(Op1, m_Undef()) || match(Op1, m_AllOnes()))
return Constant::getAllOnesValue(Op0->getType());

// X | X = X
if (Op0 == Op1)
return Op0;

// X | 0 = X
if (match(Op1, m_Zero()))
if (Op0 == Op1 || match(Op1, m_Zero()))
return Op0;

// X | -1 = -1
if (match(Op1, m_AllOnes()))
return Op1;

// A | ~A = ~A | A = -1
if (match(Op0, m_Not(m_Specific(Op1))) ||
match(Op1, m_Not(m_Specific(Op0))))
Expand Down
3 changes: 1 addition & 2 deletions test/Transforms/InstSimplify/or.ll
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ define i32 @all_ones(i32 %A) {

define <3 x i8> @all_ones_vec_with_undef_elt(<3 x i8> %A) {
; CHECK-LABEL: @all_ones_vec_with_undef_elt(
; CHECK-NEXT: [[B:%.*]] = or <3 x i8> [[A:%.*]], <i8 -1, i8 undef, i8 -1>
; CHECK-NEXT: ret <3 x i8> [[B]]
; CHECK-NEXT: ret <3 x i8> <i8 -1, i8 -1, i8 -1>
;
%B = or <3 x i8> %A, <i8 -1, i8 undef, i8 -1>
ret <3 x i8> %B
Expand Down
3 changes: 1 addition & 2 deletions test/Transforms/InstSimplify/shr-nop.ll
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,7 @@ define i1 @ashr_ne_first_zero(i8 %a) {

define <3 x i8> @ashr_all_ones_vec_with_undef_elts(<3 x i8> %x, <3 x i8> %y) {
; CHECK-LABEL: @ashr_all_ones_vec_with_undef_elts(
; CHECK-NEXT: [[SH:%.*]] = ashr <3 x i8> <i8 undef, i8 -1, i8 undef>, [[Y:%.*]]
; CHECK-NEXT: ret <3 x i8> [[SH]]
; CHECK-NEXT: ret <3 x i8> <i8 -1, i8 -1, i8 -1>
;
%sh = ashr <3 x i8> <i8 undef, i8 -1, i8 undef>, %y
ret <3 x i8> %sh
Expand Down

0 comments on commit 5563781

Please sign in to comment.