<div dir="auto">Could use a comment about what the transform is doing and why we're disallowing the ppc float, otherwise LGTM. </div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Apr 7, 2020, 5:23 AM Sanjay Patel via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">spatel created this revision.<br>
spatel added reviewers: echristo, kbarton.<br>
Herald added subscribers: hiraditya, mcrosier.<br>
<br>
Based on the post-commit comments for rG0f56bbc <<a href="https://reviews.llvm.org/rG0f56bbc1a5b2ddc881d1c55c9024b9c473dac6f0" rel="noreferrer noreferrer" target="_blank">https://reviews.llvm.org/rG0f56bbc1a5b2ddc881d1c55c9024b9c473dac6f0</a>>, there might be a problem with this transform:<br>
<br>
  (bitcast (fpext/fptrunc X)) to iX) < 0 --> (bitcast X to iY) < 0<br>
<br>
...and the ppc_fp128 data type, so conservatively bypass if we are bitcasting a ppc_fp128.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D77642" rel="noreferrer noreferrer" target="_blank">https://reviews.llvm.org/D77642</a><br>
<br>
Files:<br>
  llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp<br>
  llvm/test/Transforms/InstCombine/icmp.ll<br>
<br>
<br>
Index: llvm/test/Transforms/InstCombine/icmp.ll<br>
===================================================================<br>
--- llvm/test/Transforms/InstCombine/icmp.ll<br>
+++ llvm/test/Transforms/InstCombine/icmp.ll<br>
@@ -3700,8 +3700,9 @@<br>
<br>
 define i1 @signbit_bitcast_fpext_ppc_fp128(float %x) {<br>
 ; CHECK-LABEL: @signbit_bitcast_fpext_ppc_fp128(<br>
-; CHECK-NEXT:    [[TMP1:%.*]] = bitcast float [[X:%.*]] to i32<br>
-; CHECK-NEXT:    [[S4:%.*]] = icmp slt i32 [[TMP1]], 0<br>
+; CHECK-NEXT:    [[S2:%.*]] = fpext float [[X:%.*]] to ppc_fp128<br>
+; CHECK-NEXT:    [[S3:%.*]] = bitcast ppc_fp128 [[S2]] to i128<br>
+; CHECK-NEXT:    [[S4:%.*]] = icmp slt i128 [[S3]], 0<br>
 ; CHECK-NEXT:    ret i1 [[S4]]<br>
 ;<br>
   %s2 = fpext float %x to ppc_fp128<br>
Index: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp<br>
===================================================================<br>
--- llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp<br>
+++ llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp<br>
@@ -2766,8 +2766,8 @@<br>
     // the FP cast because the FP cast does not change the sign-bit.<br>
     const APInt *C;<br>
     bool TrueIfSigned;<br>
-    if (match(Op1, m_APInt(C)) && Bitcast->hasOneUse() &&<br>
-        isSignBitCheck(Pred, *C, TrueIfSigned)) {<br>
+    if (!BCSrcOp->getType()->isPPC_FP128Ty() && match(Op1, m_APInt(C)) &&<br>
+        Bitcast->hasOneUse() && isSignBitCheck(Pred, *C, TrueIfSigned)) {<br>
       if (match(BCSrcOp, m_FPExt(m_Value(X))) ||<br>
           match(BCSrcOp, m_FPTrunc(m_Value(X)))) {<br>
         // (bitcast (fpext/fptrunc X)) to iX) < 0 --> (bitcast X to iY) < 0<br>
<br>
<br>
</blockquote></div>