[llvm] r301018 - [InstCombine] fadd double (sitofp x), y check that the promotion is valid
Andrew Adams via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 21 17:15:38 PDT 2017
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> 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
> 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/20170421/d9d5e4e1/attachment.html>
More information about the llvm-commits
mailing list