[llvm] r367226 - [ValueTracking] Remove volatile check in isGuaranteedToTransferExecutionToSuccessor

[Hideto Ueno via llvm-commits]

Author: uenoku
Date: Mon Jul 29 06:35:34 2019
New Revision: 367226

URL: http://llvm.org/viewvc/llvm-project?rev=367226&view=rev
Log:
[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

Modified:
    llvm/trunk/lib/Analysis/ValueTracking.cpp
    llvm/trunk/test/Transforms/FunctionAttrs/nonnull.ll

Modified: llvm/trunk/lib/Analysis/ValueTracking.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ValueTracking.cpp?rev=367226&r1=367225&r2=367226&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/ValueTracking.cpp (original)
+++ llvm/trunk/lib/Analysis/ValueTracking.cpp Mon Jul 29 06:35:34 2019
@@ -4221,22 +4221,9 @@ OverflowResult llvm::computeOverflowForS
 }
 
 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))

Modified: llvm/trunk/test/Transforms/FunctionAttrs/nonnull.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionAttrs/nonnull.ll?rev=367226&r1=367225&r2=367226&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/FunctionAttrs/nonnull.ll (original)
+++ llvm/trunk/test/Transforms/FunctionAttrs/nonnull.ll Mon Jul 29 06:35:34 2019
@@ -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)