From: Hideto Ueno <uenoku.tokotoko@gmail.com>
Date: Mon, 29 Jul 2019 13:35:34 +0000 (+0000)
Subject: [ValueTracking] Remove volatile check in isGuaranteedToTransferExecutionToSuccessor
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=da3da38c85c00f93b98d7e03d2fa8d588e9377a7;p=llvm

[ValueTracking] Remove volatile check in isGuaranteedToTransferExecutionToSuccessor

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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@367226 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/Analysis/ValueTracking.cpp b/lib/Analysis/ValueTracking.cpp
index c70906dcc62..4272f69ef81 100644
--- a/lib/Analysis/ValueTracking.cpp
+++ b/lib/Analysis/ValueTracking.cpp
@@ -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))
diff --git a/test/Transforms/FunctionAttrs/nonnull.ll b/test/Transforms/FunctionAttrs/nonnull.ll
index 141fa58db46..a236aeab8ac 100644
--- a/test/Transforms/FunctionAttrs/nonnull.ll
+++ b/test/Transforms/FunctionAttrs/nonnull.ll
@@ -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)