[PatternMatch, InstSimplify] enhance m_AllOnes() to ignore undef elements 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.

llvm-svn: 325466

diff --git a/llvm/include/llvm/IR/PatternMatch.h b/llvm/include/llvm/IR/PatternMatch.h
index 36b4a45..2c579e4 100644 (file)
--- a/llvm/include/llvm/IR/PatternMatch.h
+++ b/llvm/include/llvm/IR/PatternMatch.h
@@ -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))
@@ -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(); }
 };

diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index 34a10c5..a424e69 100644 (file)
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -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;
@@ -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))))

diff --git a/llvm/test/Transforms/InstSimplify/or.ll b/llvm/test/Transforms/InstSimplify/or.ll
index e9b61b5..20f6776 100644 (file)
--- a/llvm/test/Transforms/InstSimplify/or.ll
+++ b/llvm/test/Transforms/InstSimplify/or.ll
@@ -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

diff --git a/llvm/test/Transforms/InstSimplify/shr-nop.ll b/llvm/test/Transforms/InstSimplify/shr-nop.ll
index c0bdcaa..319d319 100644 (file)
--- a/llvm/test/Transforms/InstSimplify/shr-nop.ll
+++ b/llvm/test/Transforms/InstSimplify/shr-nop.ll
@@ -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