[PATCH] D66232: [InstCombine] Try to reuse constant from select in leading comparison

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:1297-1300
+  // If the comparison is a sign-test, don't touch it.
+  // FIXME: are there more magic values we must not touch?
+  if (ICmpInst::isSigned(Pred) && C0->isNullValue())
+    return nullptr;
----------------
spatel wrote:
> Is this check to avoid an infinite loop or some other reason?
> 
> It seems strange that we need only that specific check. Maybe this should re-use this code from InstCombineCompares.cpp:
>   static bool isSignTest(ICmpInst::Predicate &Pred, const APInt &C)
> 
> or llvm::decomposeBitTestICmp()?
> 
> If I'm seeing it correctly, we would transform this case (and that's ok?):
>   define i32 @x_is_not_negative(i32 %x, i32 %y) {
>     %t = icmp sgt i32 %x, -1
>     %r = select i1 %t, i32 0, i32 %y
>     ret i32 %r
>   }
> 
> Is this check to avoid an infinite loop?

Initially - yes.
But weird, if i drop that check the original testcase no longer hangs.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66232/new/

https://reviews.llvm.org/D66232