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