[llvm] r325466 - [PatternMatch, InstSimplify] enhance m_AllOnes() to ignore undef elements in vectors

Sanjay Patel via llvm-commits llvm-commits at lists.llvm.org
Sun Feb 18 10:31:37 PST 2018


Disregard the warning about the existing matchers. They're capturing the
APInt value, not the Constant wrapper object. That should be safe.

On Sun, Feb 18, 2018 at 11:05 AM, Sanjay Patel via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: spatel
> Date: Sun Feb 18 10:05:08 2018
> New Revision: 325466
>
> URL: http://llvm.org/viewvc/llvm-project?rev=325466&view=rev
> Log:
> [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.
>
>
> Modified:
>     llvm/trunk/include/llvm/IR/PatternMatch.h
>     llvm/trunk/lib/Analysis/InstructionSimplify.cpp
>     llvm/trunk/test/Transforms/InstSimplify/or.ll
>     llvm/trunk/test/Transforms/InstSimplify/shr-nop.ll
>
> Modified: llvm/trunk/include/llvm/IR/PatternMatch.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/
> llvm/IR/PatternMatch.h?rev=325466&r1=325465&r2=325466&view=diff
> ============================================================
> ==================
> --- llvm/trunk/include/llvm/IR/PatternMatch.h (original)
> +++ llvm/trunk/include/llvm/IR/PatternMatch.h Sun Feb 18 10:05:08 2018
> @@ -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
>  //
>  ////////////////////////////////////////////////////////////
> ///////////////////
>
> +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(); }
>  };
>
> Modified: llvm/trunk/lib/Analysis/InstructionSimplify.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/
> Analysis/InstructionSimplify.cpp?rev=325466&r1=325465&r2=325466&view=diff
> ============================================================
> ==================
> --- llvm/trunk/lib/Analysis/InstructionSimplify.cpp (original)
> +++ llvm/trunk/lib/Analysis/InstructionSimplify.cpp Sun Feb 18 10:05:08
> 2018
> @@ -1264,9 +1264,10 @@ static Value *SimplifyAShrInst(Value *Op
>                                      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,
>      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))))
>
> Modified: llvm/trunk/test/Transforms/InstSimplify/or.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
> Transforms/InstSimplify/or.ll?rev=325466&r1=325465&r2=325466&view=diff
> ============================================================
> ==================
> --- llvm/trunk/test/Transforms/InstSimplify/or.ll (original)
> +++ llvm/trunk/test/Transforms/InstSimplify/or.ll Sun Feb 18 10:05:08 2018
> @@ -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
>
> Modified: llvm/trunk/test/Transforms/InstSimplify/shr-nop.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
> Transforms/InstSimplify/shr-nop.ll?rev=325466&r1=325465&
> r2=325466&view=diff
> ============================================================
> ==================
> --- llvm/trunk/test/Transforms/InstSimplify/shr-nop.ll (original)
> +++ llvm/trunk/test/Transforms/InstSimplify/shr-nop.ll Sun Feb 18
> 10:05:08 2018
> @@ -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
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180218/dad192af/attachment.html>


More information about the llvm-commits mailing list