From c13175f52f47889b618d8b889283203c742dabb5 Mon Sep 17 00:00:00 2001 From: David Majnemer Date: Fri, 28 Aug 2015 21:13:39 +0000 Subject: [PATCH] Revert r246232 and r246304. This reverts isSafeToSpeculativelyExecute's use of ReadNone until we split ReadNone into two pieces: one attribute which reasons about how the function reasons about memory and another attribute which determines how it may be speculated, CSE'd, trap, etc. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@246331 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Analysis/ValueTracking.cpp | 43 ++++++++++++++++++++++++--- lib/CodeGen/Analysis.cpp | 22 +++++++------- test/Transforms/SimplifyCFG/speculate-math.ll | 18 ----------- 3 files changed, 51 insertions(+), 32 deletions(-) diff --git a/lib/Analysis/ValueTracking.cpp b/lib/Analysis/ValueTracking.cpp index ab6907dcbd9..352b0fbd4dc 100644 --- a/lib/Analysis/ValueTracking.cpp +++ b/lib/Analysis/ValueTracking.cpp @@ -3147,11 +3147,46 @@ bool llvm::isSafeToSpeculativelyExecute(const Value *V, LI->getPointerOperand(), LI->getAlignment(), DL, CtxI, DT, TLI); } case Instruction::Call: { - auto *CI = cast(Inst); - if (CI->doesNotAccessMemory() && !CI->isMustTailCall()) - return true; + 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; + } + } return false; // The called function could have undefined behavior or - // side-effects. + // side-effects, even if marked readnone nounwind. } case Instruction::VAArg: case Instruction::Alloca: diff --git a/lib/CodeGen/Analysis.cpp b/lib/CodeGen/Analysis.cpp index 33ad88358e5..98d4c8afc7b 100644 --- a/lib/CodeGen/Analysis.cpp +++ b/lib/CodeGen/Analysis.cpp @@ -506,16 +506,18 @@ bool llvm::isInTailCallPosition(ImmutableCallSite CS, const TargetMachine &TM) { // If I will have a chain, make sure no other instruction that will have a // chain interposes between I and the return. - for (BasicBlock::const_iterator BBI = std::prev(ExitBB->end(), 2);; --BBI) { - if (&*BBI == I) - break; - // Debug info intrinsics do not get in the way of tail call optimization. - if (isa(BBI)) - continue; - if (BBI->mayHaveSideEffects() || BBI->mayReadFromMemory() || - !isSafeToSpeculativelyExecute(BBI)) - return false; - } + if (I->mayHaveSideEffects() || I->mayReadFromMemory() || + !isSafeToSpeculativelyExecute(I)) + for (BasicBlock::const_iterator BBI = std::prev(ExitBB->end(), 2);; --BBI) { + if (&*BBI == I) + break; + // Debug info intrinsics do not get in the way of tail call optimization. + if (isa(BBI)) + continue; + if (BBI->mayHaveSideEffects() || BBI->mayReadFromMemory() || + !isSafeToSpeculativelyExecute(BBI)) + return false; + } const Function *F = ExitBB->getParent(); return returnTypeIsEligibleForTailCall( diff --git a/test/Transforms/SimplifyCFG/speculate-math.ll b/test/Transforms/SimplifyCFG/speculate-math.ll index 96bdd1a9048..0ba93d29117 100644 --- a/test/Transforms/SimplifyCFG/speculate-math.ll +++ b/test/Transforms/SimplifyCFG/speculate-math.ll @@ -6,7 +6,6 @@ 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 @@ -109,20 +108,3 @@ 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 -} -- 2.11.0