[PATCH] D15249: [InstCombine] Call getCmpPredicateForMinMax only with a valid SPF

James Molloy via llvm-commits llvm-commits at lists.llvm.org
Sat Dec 5 11:20:00 PST 2015


lGTM, thanks for fixing this before I got to it myself!
On Fri, 4 Dec 2015 at 23:36, Sanjoy Das via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> sanjoy created this revision.
> sanjoy added a reviewer: majnemer.
> sanjoy added a subscriber: llvm-commits.
>
> There are `SelectPatternFlavor`s that don't represent min or max idioms,
> and we should not be passing those to `getCmpPredicateForMinMax`.
>
> Fixes PR25745.
>
> http://reviews.llvm.org/D15249
>
> Files:
>   include/llvm/Analysis/ValueTracking.h
>   lib/Transforms/InstCombine/InstCombineSelect.cpp
>   test/Transforms/InstCombine/pr25745.ll
>
> Index: test/Transforms/InstCombine/pr25745.ll
> ===================================================================
> --- /dev/null
> +++ test/Transforms/InstCombine/pr25745.ll
> @@ -0,0 +1,20 @@
> +; RUN: opt -S -instcombine < %s | FileCheck %s
> +
> +; Checking for a crash
> +
> +declare void @use.i1(i1 %val)
> +declare void @use.i64(i64 %val)
> +
> +define i64 @f(i32 %x) {
> +; CHECK-LABEL: @f(
> + entry:
> +  %x.wide = sext i32 %x to i64
> +  %minus.x = sub i32 0, %x
> +  %minus.x.wide = sext i32 %minus.x to i64
> +  %c = icmp slt i32 %x, 0
> +  %val = select i1 %c, i64 %x.wide, i64 %minus.x.wide
> +  call void @use.i1(i1 %c)
> +  call void @use.i64(i64 %x.wide)
> +  ret i64 %val
> +; CHECK: ret i64 %val
> +}
> Index: lib/Transforms/InstCombine/InstCombineSelect.cpp
> ===================================================================
> --- lib/Transforms/InstCombine/InstCombineSelect.cpp
> +++ lib/Transforms/InstCombine/InstCombineSelect.cpp
> @@ -1070,7 +1070,7 @@
>      SelectPatternResult SPR = matchSelectPattern(&SI, LHS, RHS, &CastOp);
>      auto SPF = SPR.Flavor;
>
> -    if (SPF) {
> +    if (SelectPatternResult::isMinOrMax(SPF)) {
>        // Canonicalize so that type casts are outside select patterns.
>        if (LHS->getType()->getPrimitiveSizeInBits() !=
>            SI.getType()->getPrimitiveSizeInBits()) {
> @@ -1091,11 +1091,15 @@
>                                             SI.getType());
>          return ReplaceInstUsesWith(SI, NewSI);
>        }
> +    }
>
> +    if (SPF) {
>        // MAX(MAX(a, b), a) -> MAX(a, b)
>        // MIN(MIN(a, b), a) -> MIN(a, b)
>        // MAX(MIN(a, b), a) -> a
>        // MIN(MAX(a, b), a) -> a
> +      // ABS(ABS(a)) -> ABS(a)
> +      // NABS(NABS(a)) -> NABS(a)
>        if (SelectPatternFlavor SPF2 = matchSelectPattern(LHS, LHS2,
> RHS2).Flavor)
>          if (Instruction *R =
> FoldSPFofSPF(cast<Instruction>(LHS),SPF2,LHS2,RHS2,
>                                            SI, SPF, RHS))
> Index: include/llvm/Analysis/ValueTracking.h
> ===================================================================
> --- include/llvm/Analysis/ValueTracking.h
> +++ include/llvm/Analysis/ValueTracking.h
> @@ -412,6 +412,11 @@
>      bool Ordered;               /// When implementing this min/max
> pattern as
>                                  /// fcmp; select, does the fcmp have to be
>                                  /// ordered?
> +
> +    /// \brief Return true if \p SPF is a min or a max pattern.
> +    static bool isMinOrMax(SelectPatternFlavor SPF) {
> +      return !(SPF == SPF_UNKNOWN || SPF == SPF_ABS || SPF == SPF_NABS);
> +    }
>    };
>    /// Pattern match integer [SU]MIN, [SU]MAX and ABS idioms, returning
> the kind
>    /// and providing the out parameter results if we successfully match.
>
>
> _______________________________________________
> 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/20151205/2c9026c2/attachment.html>


More information about the llvm-commits mailing list