<div dir="ltr">I'm now encountering: <a href="https://bugs.llvm.org//show_bug.cgi?id=32740">https://bugs.llvm.org//show_bug.cgi?id=32740</a><div><br></div><div>It seems like it could plausibly be related to this commit?</div><div><br></div><div>- Andrew</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Apr 21, 2017 at 11:45 AM, Artur Pilipenko via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: apilipenko<br>
Date: Fri Apr 21 13:45:25 2017<br>
New Revision: 301018<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=301018&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=301018&view=rev</a><br>
Log:<br>
[InstCombine] fadd double (sitofp x), y check that the promotion is valid<br>
<br>
Doing these transformations check that the result of integer addition is representable in the FP type.<br>
<br>
(fadd double (sitofp x), fpcst) --> (sitofp (add int x, intcst))<br>
(fadd double (sitofp x), (sitofp y)) --> (sitofp (add int x, y))<br>
<br>
This is a fix for <a href="https://bugs.llvm.org//show_bug.cgi?id=27036" rel="noreferrer" target="_blank">https://bugs.llvm.org//show_<wbr>bug.cgi?id=27036</a><br>
<br>
Reviewed By: andrew.w.kaylor, scanon, spatel<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D31182" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D31182</a><br>
<br>
Modified:<br>
llvm/trunk/lib/Transforms/<wbr>InstCombine/InstCombineAddSub.<wbr>cpp<br>
llvm/trunk/test/Transforms/<wbr>InstCombine/add-sitofp.ll<br>
<br>
Modified: llvm/trunk/lib/Transforms/<wbr>InstCombine/InstCombineAddSub.<wbr>cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp?rev=301018&r1=301017&r2=301018&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/<wbr>Transforms/InstCombine/<wbr>InstCombineAddSub.cpp?rev=<wbr>301018&r1=301017&r2=301018&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/lib/Transforms/<wbr>InstCombine/InstCombineAddSub.<wbr>cpp (original)<br>
+++ llvm/trunk/lib/Transforms/<wbr>InstCombine/InstCombineAddSub.<wbr>cpp Fri Apr 21 13:45:25 2017<br>
@@ -1385,39 +1385,55 @@ Instruction *InstCombiner::visitFAdd(Bin<br>
// integer add followed by a promotion.<br>
if (SIToFPInst *LHSConv = dyn_cast<SIToFPInst>(LHS)) {<br>
Value *LHSIntVal = LHSConv->getOperand(0);<br>
+ Type *FPType = LHSConv->getType();<br>
+<br>
+ // TODO: This check is overly conservative. In many cases known bits<br>
+ // analysis can tell us that the result of the addition has less significant<br>
+ // bits than the integer type can hold.<br>
+ auto IsValidPromotion = [](Type *FTy, Type *ITy) {<br>
+ // Do we have enough bits in the significand to represent the result of<br>
+ // the integer addition?<br>
+ unsigned MaxRepresentableBits =<br>
+ APFloat::semanticsPrecision(<wbr>FTy->getFltSemantics());<br>
+ return ITy->getIntegerBitWidth() <= MaxRepresentableBits;<br>
+ };<br>
<br>
// (fadd double (sitofp x), fpcst) --> (sitofp (add int x, intcst))<br>
// ... if the constant fits in the integer value. This is useful for things<br>
// like (double)(x & 1234) + 4.0 -> (double)((X & 1234)+4) which no longer<br>
// requires a constant pool load, and generally allows the add to be better<br>
// instcombined.<br>
- if (ConstantFP *CFP = dyn_cast<ConstantFP>(RHS)) {<br>
- Constant *CI =<br>
- ConstantExpr::getFPToSI(CFP, LHSIntVal->getType());<br>
- if (LHSConv->hasOneUse() &&<br>
- ConstantExpr::getSIToFP(CI, I.getType()) == CFP &&<br>
- WillNotOverflowSignedAdd(<wbr>LHSIntVal, CI, I)) {<br>
- // Insert the new integer add.<br>
- Value *NewAdd = Builder->CreateNSWAdd(<wbr>LHSIntVal,<br>
- CI, "addconv");<br>
- return new SIToFPInst(NewAdd, I.getType());<br>
+ if (ConstantFP *CFP = dyn_cast<ConstantFP>(RHS))<br>
+ if (IsValidPromotion(FPType, LHSIntVal->getType())) {<br>
+ Constant *CI =<br>
+ ConstantExpr::getFPToSI(CFP, LHSIntVal->getType());<br>
+ if (LHSConv->hasOneUse() &&<br>
+ ConstantExpr::getSIToFP(CI, I.getType()) == CFP &&<br>
+ WillNotOverflowSignedAdd(<wbr>LHSIntVal, CI, I)) {<br>
+ // Insert the new integer add.<br>
+ Value *NewAdd = Builder->CreateNSWAdd(<wbr>LHSIntVal,<br>
+ CI, "addconv");<br>
+ return new SIToFPInst(NewAdd, I.getType());<br>
+ }<br>
}<br>
- }<br>
<br>
// (fadd double (sitofp x), (sitofp y)) --> (sitofp (add int x, y))<br>
if (SIToFPInst *RHSConv = dyn_cast<SIToFPInst>(RHS)) {<br>
Value *RHSIntVal = RHSConv->getOperand(0);<br>
-<br>
- // Only do this if x/y have the same type, if at least one of them has a<br>
- // single use (so we don't increase the number of int->fp conversions),<br>
- // and if the integer add will not overflow.<br>
- if (LHSIntVal->getType() == RHSIntVal->getType() &&<br>
- (LHSConv->hasOneUse() || RHSConv->hasOneUse()) &&<br>
- WillNotOverflowSignedAdd(<wbr>LHSIntVal, RHSIntVal, I)) {<br>
- // Insert the new integer add.<br>
- Value *NewAdd = Builder->CreateNSWAdd(<wbr>LHSIntVal,<br>
- RHSIntVal, "addconv");<br>
- return new SIToFPInst(NewAdd, I.getType());<br>
+ // It's enough to check LHS types only because we require int types to<br>
+ // be the same for this transform.<br>
+ if (IsValidPromotion(FPType, LHSIntVal->getType())) {<br>
+ // Only do this if x/y have the same type, if at least one of them has a<br>
+ // single use (so we don't increase the number of int->fp conversions),<br>
+ // and if the integer add will not overflow.<br>
+ if (LHSIntVal->getType() == RHSIntVal->getType() &&<br>
+ (LHSConv->hasOneUse() || RHSConv->hasOneUse()) &&<br>
+ WillNotOverflowSignedAdd(<wbr>LHSIntVal, RHSIntVal, I)) {<br>
+ // Insert the new integer add.<br>
+ Value *NewAdd = Builder->CreateNSWAdd(<wbr>LHSIntVal,<br>
+ RHSIntVal, "addconv");<br>
+ return new SIToFPInst(NewAdd, I.getType());<br>
+ }<br>
}<br>
}<br>
}<br>
<br>
Modified: llvm/trunk/test/Transforms/<wbr>InstCombine/add-sitofp.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/add-sitofp.ll?rev=301018&r1=301017&r2=301018&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/test/<wbr>Transforms/InstCombine/add-<wbr>sitofp.ll?rev=301018&r1=<wbr>301017&r2=301018&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/Transforms/<wbr>InstCombine/add-sitofp.ll (original)<br>
+++ llvm/trunk/test/Transforms/<wbr>InstCombine/add-sitofp.ll Fri Apr 21 13:45:25 2017<br>
@@ -15,3 +15,88 @@ define double @x(i32 %a, i32 %b) {<br>
%p = fadd double %o, 1.0<br>
ret double %p<br>
}<br>
+<br>
+define double @test(i32 %a) {<br>
+; CHECK-LABEL: @test(<br>
+; CHECK-NEXT: [[A_AND:%.*]] = and i32 [[A:%.*]], 1073741823<br>
+; CHECK-NEXT: [[ADDCONV:%.*]] = add nuw nsw i32 [[A_AND]], 1<br>
+; CHECK-NEXT: [[RES:%.*]] = sitofp i32 [[ADDCONV]] to double<br>
+; CHECK-NEXT: ret double [[RES]]<br>
+;<br>
+ ; Drop two highest bits to guarantee that %a + 1 doesn't overflow<br>
+ %a_and = and i32 %a, 1073741823<br>
+ %a_and_fp = sitofp i32 %a_and to double<br>
+ %res = fadd double %a_and_fp, 1.0<br>
+ ret double %res<br>
+}<br>
+<br>
+define float @test_neg(i32 %a) {<br>
+; CHECK-LABEL: @test_neg(<br>
+; CHECK-NEXT: [[A_AND:%.*]] = and i32 [[A:%.*]], 1073741823<br>
+; CHECK-NEXT: [[A_AND_FP:%.*]] = sitofp i32 [[A_AND]] to float<br>
+; CHECK-NEXT: [[RES:%.*]] = fadd float [[A_AND_FP]], 1.000000e+00<br>
+; CHECK-NEXT: ret float [[RES]]<br>
+;<br>
+ ; Drop two highest bits to guarantee that %a + 1 doesn't overflow<br>
+ %a_and = and i32 %a, 1073741823<br>
+ %a_and_fp = sitofp i32 %a_and to float<br>
+ %res = fadd float %a_and_fp, 1.0<br>
+ ret float %res<br>
+}<br>
+<br>
+define double @test_2(i32 %a, i32 %b) {<br>
+; CHECK-LABEL: @test_2(<br>
+; CHECK-NEXT: [[A_AND:%.*]] = and i32 [[A:%.*]], 1073741823<br>
+; CHECK-NEXT: [[B_AND:%.*]] = and i32 [[B:%.*]], 1073741823<br>
+; CHECK-NEXT: [[ADDCONV:%.*]] = add nuw nsw i32 [[A_AND]], [[B_AND]]<br>
+; CHECK-NEXT: [[RES:%.*]] = sitofp i32 [[ADDCONV]] to double<br>
+; CHECK-NEXT: ret double [[RES]]<br>
+;<br>
+ ; Drop two highest bits to guarantee that %a + %b doesn't overflow<br>
+ %a_and = and i32 %a, 1073741823<br>
+ %b_and = and i32 %b, 1073741823<br>
+<br>
+ %a_and_fp = sitofp i32 %a_and to double<br>
+ %b_and_fp = sitofp i32 %b_and to double<br>
+<br>
+ %res = fadd double %a_and_fp, %b_and_fp<br>
+ ret double %res<br>
+}<br>
+<br>
+define float @test_2_neg(i32 %a, i32 %b) {<br>
+; CHECK-LABEL: @test_2_neg(<br>
+; CHECK-NEXT: [[A_AND:%.*]] = and i32 [[A:%.*]], 1073741823<br>
+; CHECK-NEXT: [[B_AND:%.*]] = and i32 [[B:%.*]], 1073741823<br>
+; CHECK-NEXT: [[A_AND_FP:%.*]] = sitofp i32 [[A_AND]] to float<br>
+; CHECK-NEXT: [[B_AND_FP:%.*]] = sitofp i32 [[B_AND]] to float<br>
+; CHECK-NEXT: [[RES:%.*]] = fadd float [[A_AND_FP]], [[B_AND_FP]]<br>
+; CHECK-NEXT: ret float [[RES]]<br>
+;<br>
+ ; Drop two highest bits to guarantee that %a + %b doesn't overflow<br>
+ %a_and = and i32 %a, 1073741823<br>
+ %b_and = and i32 %b, 1073741823<br>
+<br>
+ %a_and_fp = sitofp i32 %a_and to float<br>
+ %b_and_fp = sitofp i32 %b_and to float<br>
+<br>
+ %res = fadd float %a_and_fp, %b_and_fp<br>
+ ret float %res<br>
+}<br>
+<br>
+; This test demonstrates overly conservative legality check. The float addition<br>
+; can be replaced with the integer addition because the result of the operation<br>
+; can be represented in float, but we don't do that now.<br>
+define float @test_3(i32 %a, i32 %b) {<br>
+; CHECK-LABEL: @test_3(<br>
+; CHECK-NEXT: [[M:%.*]] = lshr i32 [[A:%.*]], 24<br>
+; CHECK-NEXT: [[N:%.*]] = and i32 [[M]], [[B:%.*]]<br>
+; CHECK-NEXT: [[O:%.*]] = sitofp i32 [[N]] to float<br>
+; CHECK-NEXT: [[P:%.*]] = fadd float [[O]], 1.000000e+00<br>
+; CHECK-NEXT: ret float [[P]]<br>
+;<br>
+ %m = lshr i32 %a, 24<br>
+ %n = and i32 %m, %b<br>
+ %o = sitofp i32 %n to float<br>
+ %p = fadd float %o, 1.0<br>
+ ret float %p<br>
+}<br>
<br>
<br>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>