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