From 0317b8efaff0addbf14d66583f37483c65e3f7f2 Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Thu, 8 Oct 2015 17:09:31 +0000 Subject: [PATCH] [InstCombine] transform masking off of an FP sign bit into a fabs() intrinsic call (PR24886) This is a partial fix for PR24886: https://llvm.org/bugs/show_bug.cgi?id=24886 Without this IR transform, the backend (x86 at least) was producing inefficient code. This patch is making 2 assumptions: 1. The canonical form of a fabs() operation is, in fact, the LLVM fabs() intrinsic. 2. The high bit of an FP value is always the sign bit; as noted in the bug report, this isn't specified by the LangRef. Differential Revision: http://reviews.llvm.org/D13076 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@249702 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../InstCombine/InstCombineAndOrXor.cpp | 23 ++++++++-- test/Transforms/InstCombine/and2.ll | 43 +++++++++++++++++++ 2 files changed, 62 insertions(+), 4 deletions(-) diff --git a/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp index d6e87b61177..f72089e6c8e 100644 --- a/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp +++ b/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp @@ -1488,14 +1488,15 @@ Instruction *InstCombiner::visitAnd(BinaryOperator &I) { return ReplaceInstUsesWith(I, Res); - // fold (and (cast A), (cast B)) -> (cast (and A, B)) - if (CastInst *Op0C = dyn_cast(Op0)) + if (CastInst *Op0C = dyn_cast(Op0)) { + Value *Op0COp = Op0C->getOperand(0); + Type *SrcTy = Op0COp->getType(); + // fold (and (cast A), (cast B)) -> (cast (and A, B)) if (CastInst *Op1C = dyn_cast(Op1)) { - Type *SrcTy = Op0C->getOperand(0)->getType(); if (Op0C->getOpcode() == Op1C->getOpcode() && // same cast kind ? SrcTy == Op1C->getOperand(0)->getType() && SrcTy->isIntOrIntVectorTy()) { - Value *Op0COp = Op0C->getOperand(0), *Op1COp = Op1C->getOperand(0); + Value *Op1COp = Op1C->getOperand(0); // Only do this if the casts both really cause code to be generated. if (ShouldOptimizeCast(Op0C->getOpcode(), Op0COp, I.getType()) && @@ -1520,6 +1521,20 @@ Instruction *InstCombiner::visitAnd(BinaryOperator &I) { } } + // If we are masking off the sign bit of a floating-point value, convert + // this to the canonical fabs intrinsic call and cast back to integer. + // The backend should know how to optimize fabs(). + // TODO: This transform should also apply to vectors. + ConstantInt *CI; + if (isa(Op0C) && SrcTy->isFloatingPointTy() && + match(Op1, m_ConstantInt(CI)) && CI->isMaxValue(true)) { + Module *M = I.getParent()->getParent()->getParent(); + Function *Fabs = Intrinsic::getDeclaration(M, Intrinsic::fabs, SrcTy); + Value *Call = Builder->CreateCall(Fabs, Op0COp, "fabs"); + return CastInst::CreateBitOrPointerCast(Call, I.getType()); + } + } + { Value *X = nullptr; bool OpsSwapped = false; diff --git a/test/Transforms/InstCombine/and2.ll b/test/Transforms/InstCombine/and2.ll index 9be6b58979d..326bfda3855 100644 --- a/test/Transforms/InstCombine/and2.ll +++ b/test/Transforms/InstCombine/and2.ll @@ -102,3 +102,46 @@ define i64 @test10(i64 %x) { %add = add i64 %sub, %and ret i64 %add } + +define i64 @fabs_double(double %x) { +; CHECK-LABEL: @fabs_double( +; CHECK-NEXT: %fabs = call double @llvm.fabs.f64(double %x) +; CHECK-NEXT: %and = bitcast double %fabs to i64 +; CHECK-NEXT: ret i64 %and + %bc = bitcast double %x to i64 + %and = and i64 %bc, 9223372036854775807 + ret i64 %and +} + +define i64 @fabs_double_swap(double %x) { +; CHECK-LABEL: @fabs_double_swap( +; CHECK-NEXT: %fabs = call double @llvm.fabs.f64(double %x) +; CHECK-NEXT: %and = bitcast double %fabs to i64 +; CHECK-NEXT: ret i64 %and + %bc = bitcast double %x to i64 + %and = and i64 9223372036854775807, %bc + ret i64 %and +} + +define i32 @fabs_float(float %x) { +; CHECK-LABEL: @fabs_float( +; CHECK-NEXT: %fabs = call float @llvm.fabs.f32(float %x) +; CHECK-NEXT: %and = bitcast float %fabs to i32 +; CHECK-NEXT: ret i32 %and + %bc = bitcast float %x to i32 + %and = and i32 %bc, 2147483647 + ret i32 %and +} + +; Make sure that only a bitcast is transformed. + +define i64 @fabs_double_not_bitcast(double %x) { +; CHECK-LABEL: @fabs_double_not_bitcast( +; CHECK-NEXT: %bc = fptoui double %x to i64 +; CHECK-NEXT: %and = and i64 %bc, 9223372036854775807 +; CHECK-NEXT: ret i64 %and + %bc = fptoui double %x to i64 + %and = and i64 %bc, 9223372036854775807 + ret i64 %and +} + -- 2.34.1