Skip to content

Commit

Permalink
[ValueTracking] Remove volatile check in isGuaranteedToTransferExecut…
Browse files Browse the repository at this point in the history
…ionToSuccessor

Summary: As clarified in D53184, volatile load and store do not trap. Therefore, we should remove volatile checks for instructions in  `isGuaranteedToTransferExecutionToSuccessor`.

Reviewers: jdoerfert, efriedma, nikic

Reviewed By: nikic

Subscribers: hiraditya, jfb, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D65375

llvm-svn: 367226
  • Loading branch information
uenoku committed Jul 29, 2019
1 parent ff9f4b5 commit 98d281a
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 17 deletions.
17 changes: 2 additions & 15 deletions llvm/lib/Analysis/ValueTracking.cpp
Expand Up @@ -4221,22 +4221,9 @@ OverflowResult llvm::computeOverflowForSignedAdd(const Value *LHS,
}

bool llvm::isGuaranteedToTransferExecutionToSuccessor(const Instruction *I) {
// A memory operation returns normally if it isn't volatile. A volatile
// operation is allowed to trap.
//
// An atomic operation isn't guaranteed to return in a reasonable amount of
// time because it's possible for another thread to interfere with it for an
// Note: An atomic operation isn't guaranteed to return in a reasonable amount
// of time because it's possible for another thread to interfere with it for an
// arbitrary length of time, but programs aren't allowed to rely on that.
if (const LoadInst *LI = dyn_cast<LoadInst>(I))
return !LI->isVolatile();
if (const StoreInst *SI = dyn_cast<StoreInst>(I))
return !SI->isVolatile();
if (const AtomicCmpXchgInst *CXI = dyn_cast<AtomicCmpXchgInst>(I))
return !CXI->isVolatile();
if (const AtomicRMWInst *RMWI = dyn_cast<AtomicRMWInst>(I))
return !RMWI->isVolatile();
if (const MemIntrinsic *MII = dyn_cast<MemIntrinsic>(I))
return !MII->isVolatile();

// If there is no successor, then execution can't transfer to it.
if (const auto *CRI = dyn_cast<CleanupReturnInst>(I))
Expand Down
6 changes: 4 additions & 2 deletions llvm/test/Transforms/FunctionAttrs/nonnull.ll
Expand Up @@ -347,10 +347,12 @@ f:
}

; The callsite must execute in order for the attribute to transfer to the parent.
; The volatile load might trap, so there's no guarantee that we'll ever get to the call.
; The volatile load can't trap, so we can guarantee that we'll get to the call.

define i8 @parent6(i8* %a, i8* %b) {
; BOTH-LABEL: @parent6(i8* %a, i8* %b)
; FNATTR-LABEL: @parent6(i8* nonnull %a, i8* %b)
; FIXME: missing "nonnull"
; ATTRIBUTOR-LABEL: @parent6(i8* %a, i8* %b)
; BOTH-NEXT: [[C:%.*]] = load volatile i8, i8* %b
; FNATTR-NEXT: call void @use1nonnull(i8* %a)
; ATTRIBUTOR-NEXT: call void @use1nonnull(i8* nonnull %a)
Expand Down

0 comments on commit 98d281a

Please sign in to comment.