Fix an issue with isGuaranteedToTransferExecutionToSuccessor

I'm not sure if this was intentional, but today
isGuaranteedToTransferExecutionToSuccessor returns true for readonly and
argmemonly calls that may throw.  This commit changes the function to
not implicitly infer nounwind this way.

Even if we eventually specify readonly calls as not throwing,
isGuaranteedToTransferExecutionToSuccessor is not the best place to
infer that.  We should instead teach FunctionAttrs or some other such
pass to tag readonly functions / calls as nounwind instead.

llvm-svn: 290794

llvm/lib/Analysis/ValueTracking.cpp

@@ -3657,12 +3657,26 @@ bool llvm::isGuaranteedToTransferExecutionToSuccessor(const Instruction *I) {
 
   // Calls can throw, or contain an infinite loop, or kill the process.
   if (auto CS = ImmutableCallSite(I)) {
-    // Calls which don't write to arbitrary memory are safe.
-    // FIXME: Ignoring infinite loops without any side-effects is too
-    // aggressive,
-    // but it's consistent with other passes. See http://llvm.org/PR965 .
-    // FIXME: This isn't aggressive enough; a call which only writes to a
-    // global is guaranteed to return.
+    // Call sites that throw have implicit non-local control flow.
+    if (!CS.doesNotThrow())
+      return false;
+
+    // Non-throwing call sites can loop infinitely, call exit/pthread_exit
+    // etc. and thus not return.  However, LLVM already assumes that
+    //
+    //  - Thread exiting actions are modeled as writes to memory invisible to
+    //    the program.
+    //
+    //  - Loops that don't have side effects (side effects are volatile/atomic
+    //    stores and IO) always terminate (see http://llvm.org/PR965).
+    //    Furthermore IO itself is also modeled as writes to memory invisible to
+    //    the program.
+    //
+    // We rely on those assumptions here, and use the memory effects of the call
+    // target as a proxy for checking that it always returns.
+
+    // FIXME: This isn't aggressive enough; a call which only writes to a global
+    // is guaranteed to return.
     return CS.onlyReadsMemory() || CS.onlyAccessesArgMemory() ||
            match(I, m_Intrinsic<Intrinsic::assume>());
   }

llvm/unittests/Analysis/ValueTrackingTest.cpp

@@ -188,3 +188,54 @@ TEST_F(MatchSelectPatternTest, DoubleCastBad) {
   // The cast types here aren't the same, so we cannot match an UMIN.
   expectPattern({SPF_UNKNOWN, SPNB_NA, false});
 }
+
+TEST(ValueTracking, GuaranteedToTransferExecutionToSuccessor) {
+  StringRef Assembly =
+      "declare void @nounwind_readonly(i32*) nounwind readonly "
+      "declare void @nounwind_argmemonly(i32*) nounwind argmemonly "
+      "declare void @throws_but_readonly(i32*) readonly "
+      "declare void @throws_but_argmemonly(i32*) argmemonly "
+      " "
+      "declare void @unknown(i32*) "
+      " "
+      "define void @f(i32* %p) { "
+      "  call void @nounwind_readonly(i32* %p) "
+      "  call void @nounwind_argmemonly(i32* %p) "
+      "  call void @throws_but_readonly(i32* %p) "
+      "  call void @throws_but_argmemonly(i32* %p) "
+      "  call void @unknown(i32* %p) nounwind readonly "
+      "  call void @unknown(i32* %p) nounwind argmemonly "
+      "  call void @unknown(i32* %p) readonly "
+      "  call void @unknown(i32* %p) argmemonly "
+      "  ret void "
+      "} ";
+
+  LLVMContext Context;
+  SMDiagnostic Error;
+  auto M = parseAssemblyString(Assembly, Error, Context);
+  assert(M && "Bad assembly?");
+
+  auto *F = M->getFunction("f");
+  assert(F && "Bad assembly?");
+
+  auto &BB = F->getEntryBlock();
+  ArrayRef<bool> ExpectedAnswers = {
+      true,  // call void @nounwind_readonly(i32* %p)
+      true,  // call void @nounwind_argmemonly(i32* %p)
+      false, // call void @throws_but_readonly(i32* %p)
+      false, // call void @throws_but_argmemonly(i32* %p)
+      true,  // call void @unknown(i32* %p) nounwind readonly
+      true,  // call void @unknown(i32* %p) nounwind argmemonly
+      false, // call void @unknown(i32* %p) readonly
+      false, // call void @unknown(i32* %p) argmemonly
+      false, // ret void
+  };
+
+  int Index = 0;
+  for (auto &I : BB) {
+    EXPECT_EQ(isGuaranteedToTransferExecutionToSuccessor(&I),
+              ExpectedAnswers[Index])
+        << "Incorrect answer at instruction " << Index << " = " << I;
+    Index++;
+  }
+}