Use "willreturn" function attribute in isGuaranteedToTransferExecutionToSuccessor

[uenoku]

Summary

"willreturn" function attribute guarantees a function to come back to the call site. Therefore, this attribute can be used in isGuaranteedToTransferExecutionToSuccessor.

[jdoerfert]

Could you run this with the LLVM test suite [0]?
Do you see a difference in statistics, compile time and/or runtime?

If you don't have a computer to do so, let me know.

[0] http://llvm.org/docs/lnt/quickstart.html

[jdoerfert]

llvm/lib/Analysis/ValueTracking.cpp

4290

Can you keep and modify the comment instead? There are other possibilities specified to improve this and/or the willreturn deduction.

[uenoku]

Address comment.

[uenoku]

In D63372#1547756, @jdoerfert wrote:

Could you run this with the LLVM test suite [0]?
Do you see a difference in statistics, compile time and/or runtime?

If you don't have a computer to do so, let me know.

[0] http://llvm.org/docs/lnt/quickstart.html

I ran LLVM test suite but there was no significant difference in both execution time and runtime.
I want to see stats (NumFnWillReturn and etc.) but I don't know how to gather stats. I ran with TEST_SUITE_COLLECT_STATS=ON but I couldn't merge them. Can you tell me?

[jdoerfert]

In D63372#1551988, @uenoku wrote:

In D63372#1547756, @jdoerfert wrote:

Could you run this with the LLVM test suite [0]?
Do you see a difference in statistics, compile time and/or runtime?

If you don't have a computer to do so, let me know.

[0] http://llvm.org/docs/lnt/quickstart.html

I ran LLVM test suite but there was no significant difference in both execution time and runtime.
I want to see stats (NumFnWillReturn and etc.) but I don't know how to gather stats. I ran with TEST_SUITE_COLLECT_STATS=ON but I couldn't merge them. Can you tell me?

I have scripts for that, I'll send them tomorrow, traveling today :(

[jdoerfert]

I think this looks fine with one request for change: We "decided" to use enum attributes not string attributes (for willreturn and others).

To seem real improvements we might need to use this function more (we will soon in the Attributor). Just to make sure, you enabled the attributor and willreturn deduction for your test, correct?

[uenoku]

I think this looks fine with one request for change: We "decided" to use enum attributes not string attributes (for willreturn and others).

I'll fix it.

To seem real improvements we might need to use this function more (we will soon in the Attributor). Just to make sure, you enabled the attributor and willreturn deduction for your test, correct?

Yes. I confirmed in the report that willreturn is actually deduced.

[uenoku]

Use enum attr.

llvm/lib/Analysis/ValueTracking.cpp

if (isa<UnreachableInst>(I))
     return false;
 
   // Calls can throw, or contain an infinite loop, or kill the process.
   if (auto CS = ImmutableCallSite(I)) {
     // 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.
+    // A function which doens't throw and has "willreturn" attribute will
+    // always return.
+    if (CS.hasFnAttr("willreturn"))
+      return true;
 
     // 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>()) ||
            match(I, m_Intrinsic<Intrinsic::sideeffect>()) ||
            match(I, m_Intrinsic<Intrinsic::experimental_widenable_condition>());
   }
 

llvm/unittests/Analysis/ValueTrackingTest.cpp

 }
 
 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 @nounwind_willreturn(i32*) nounwind \"willreturn\""
       " "
       "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 "
+      "  call void @nounwind_willreturn(i32* %p)"
       "  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();
   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
+      true,  // call void @nounwind_willreturn(i32* %p)
       false, // ret void
   };
 
   int Index = 0;
   for (auto &I : BB) {
     EXPECT_EQ(isGuaranteedToTransferExecutionToSuccessor(&I),
               ExpectedAnswers[Index])
         << "Incorrect answer at instruction " << Index << " = " << I;