The following test case is tripping an infinite loop in InstCombine. I've been unable to track down the root cause yet. This issue was found in release/13.x, but Godbolt shows it failing in trunk as well. https://godbolt.org/z/xKasbGheK ; RUN: opt -O3 -instcombine -S < %s define i64 @infloop(double %d, i64 %a) { %1 = fptosi double %d to i32 %2 = call i32 @llvm.smax.i32(i32 %1, i32 0) %3 = icmp slt i32 %2, 2 %4 = zext i1 %3 to i32 %5 = select i1 %3, i32 %2, i32 1 %6 = zext i32 %5 to i64 %7 = mul i64 %6, 4 %8 = add i64 %a, %7 ret i64 %8 }
Reduced: declare i32 @llvm.smax.i32(i32, i32); define i32 @infloop(i32 %x, double %d, i64 %a) { %t2 = call i32 @llvm.smax.i32(i32 %x, i32 0) %t3 = icmp slt i32 %t2, 2 %t5 = select i1 %t3, i32 %t2, i32 1 ret i32 %t5 } There are opposing transforms somewhere under visitICmp. I'll try to find and fix.
https://reviews.llvm.org/rGb267d3ce8def Leaving open as candidate for blocking 13.0 release.
If backporting, we'll also need this patch to avoid breaking tests: https://reviews.llvm.org/rGe260e10c4a21
(In reply to Sanjay Patel from comment #3) > If backporting, we'll also need this patch to avoid breaking tests: > https://reviews.llvm.org/rGe260e10c4a21 And I almost forgot already, but that one was on top of adding tests: https://reviews.llvm.org/rG9b942a545cb5
Thanks for the quick response, Sanjay. I appreciate it.
(In reply to Cameron McInally from comment #5) > Thanks for the quick response, Sanjay. I appreciate it. Some day, we'll canonicalize to the intrinsics and get out of this mess. :) Just curious - did this show up in real code or was it found by fuzzing of some kind?
(In reply to Sanjay Patel from comment #6) > Just curious - did this show up in real code or was it found by fuzzing of > some kind? It showed up in a handful of SPEC codes (e.g. omnetpp, wrf, blender). We don't use the Clang frontend though. And we also do heavy vectorization before opt is called.
(In reply to Cameron McInally from comment #7) > It showed up in a handful of SPEC codes (e.g. omnetpp, wrf, blender). We > don't use the Clang frontend though. And we also do heavy vectorization > before opt is called. Thanks - that sounds like good incentive to try to backport this to the release. After reviewing my patch, I think that we do *not* need to backport the changes to InstSimplify to get the InstCombine fix to patch cleanly. So to be clear, I expect that b267d3ce8def can be backported alone if that is viewed as a less risky/more direct bug fix. But taking 9b942a545cb5 and e260e10c4a21 (InstSimplify) would get us more in line with the current 'main' branch.
Merged: 5b60faae3f10