[PATCH] D65375: [ValueTracking] Remove volatile check in isGuaranteedToTransferExecutionToSuccessor

[Hideto Ueno via Phabricator via llvm-commits]

uenoku updated this revision to Diff 212159.
uenoku added a comment.
Herald added subscribers: llvm-commits, jfb, hiraditya.
Herald added a project: LLVM.

Address comment.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65375/new/

https://reviews.llvm.org/D65375

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


Index: llvm/test/Transforms/FunctionAttrs/nonnull.ll
===================================================================
--- llvm/test/Transforms/FunctionAttrs/nonnull.ll
+++ llvm/test/Transforms/FunctionAttrs/nonnull.ll
@@ -347,10 +347,12 @@
 }
 
 ; 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)
Index: llvm/lib/Analysis/ValueTracking.cpp
===================================================================
--- llvm/lib/Analysis/ValueTracking.cpp
+++ llvm/lib/Analysis/ValueTracking.cpp
@@ -4221,22 +4221,9 @@
 }
 
 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))


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D65375.212159.patch
Type: text/x-patch
Size: 2272 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190729/4129d071/attachment.bin>