From 0293704be2e95e0c3506eec2b4334577717a529f Mon Sep 17 00:00:00 2001 From: David Majnemer Date: Thu, 27 Aug 2015 23:03:01 +0000 Subject: [PATCH] [ValueTracking] readnone CallInsts are fair game for speculation Any call which is side effect free is trivially OK to speculate. We already had similar logic in EarlyCSE and GVN but we were missing it from isSafeToSpeculativelyExecute. This fixes PR24601. llvm-svn: 246232 --- llvm/lib/Analysis/ValueTracking.cpp | 42 ++----------------- .../Transforms/SimplifyCFG/speculate-math.ll | 18 ++++++++ 2 files changed, 21 insertions(+), 39 deletions(-) diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp index 352b0fbd4dcb..d3d71182b762 100644 --- a/llvm/lib/Analysis/ValueTracking.cpp +++ b/llvm/lib/Analysis/ValueTracking.cpp @@ -3147,46 +3147,10 @@ bool llvm::isSafeToSpeculativelyExecute(const Value *V, LI->getPointerOperand(), LI->getAlignment(), DL, CtxI, DT, TLI); } case Instruction::Call: { - if (const IntrinsicInst *II = dyn_cast(Inst)) { - switch (II->getIntrinsicID()) { - // These synthetic intrinsics have no side-effects and just mark - // information about their operands. - // FIXME: There are other no-op synthetic instructions that potentially - // should be considered at least *safe* to speculate... - case Intrinsic::dbg_declare: - case Intrinsic::dbg_value: - return true; - - case Intrinsic::bswap: - case Intrinsic::ctlz: - case Intrinsic::ctpop: - case Intrinsic::cttz: - case Intrinsic::objectsize: - case Intrinsic::sadd_with_overflow: - case Intrinsic::smul_with_overflow: - case Intrinsic::ssub_with_overflow: - case Intrinsic::uadd_with_overflow: - case Intrinsic::umul_with_overflow: - case Intrinsic::usub_with_overflow: - return true; - // Sqrt should be OK, since the llvm sqrt intrinsic isn't defined to set - // errno like libm sqrt would. - case Intrinsic::sqrt: - case Intrinsic::fma: - case Intrinsic::fmuladd: - case Intrinsic::fabs: - case Intrinsic::minnum: - case Intrinsic::maxnum: - return true; - // TODO: some fp intrinsics are marked as having the same error handling - // as libm. They're safe to speculate when they won't error. - // TODO: are convert_{from,to}_fp16 safe? - // TODO: can we list target-specific intrinsics here? - default: break; - } - } + if (cast(Inst)->doesNotAccessMemory()) + return true; return false; // The called function could have undefined behavior or - // side-effects, even if marked readnone nounwind. + // side-effects. } case Instruction::VAArg: case Instruction::Alloca: diff --git a/llvm/test/Transforms/SimplifyCFG/speculate-math.ll b/llvm/test/Transforms/SimplifyCFG/speculate-math.ll index 0ba93d29117a..96bdd1a90485 100644 --- a/llvm/test/Transforms/SimplifyCFG/speculate-math.ll +++ b/llvm/test/Transforms/SimplifyCFG/speculate-math.ll @@ -6,6 +6,7 @@ declare float @llvm.fmuladd.f32(float, float, float) nounwind readonly declare float @llvm.fabs.f32(float) nounwind readonly declare float @llvm.minnum.f32(float, float) nounwind readonly declare float @llvm.maxnum.f32(float, float) nounwind readonly +declare float @llvm.copysign.f32(float, float) nounwind readonly ; CHECK-LABEL: @sqrt_test( ; CHECK: select @@ -108,3 +109,20 @@ test_maxnum.exit: ; preds = %cond.else.i, %ent store float %cond.i, float addrspace(1)* %out, align 4 ret void } + +; CHECK-LABEL: @copysign_test( +; CHECK: select +define void @copysign_test(float addrspace(1)* noalias nocapture %out, float %a, float %b) nounwind { +entry: + %cmp.i = fcmp olt float %a, 0.000000e+00 + br i1 %cmp.i, label %test_copysign.exit, label %cond.else.i + +cond.else.i: ; preds = %entry + %0 = tail call float @llvm.copysign.f32(float %a, float %b) nounwind readnone + br label %test_copysign.exit + +test_copysign.exit: ; preds = %cond.else.i, %entry + %cond.i = phi float [ %0, %cond.else.i ], [ 0x7FF8000000000000, %entry ] + store float %cond.i, float addrspace(1)* %out, align 4 + ret void +}