[Inliner] Add return attributes to callsites not marked `willreturn`/`nounwind`

[goldstein.w.n]

Summary

The actual callsite we are adding to doesn't need to be
willreturn/nounwind, only ever instructions between the callsite
and the return.

[goldstein.w.n]

ping.

[nikic]

llvm/lib/Transforms/Utils/InlineFunction.cpp

1340

Can't happen? You can assert this instead.

1399

Why do we need the unwind check? If the function unwinds, the attribute doesn't matter. If it doesn't unwind, then it will reach the nonnull.

[goldstein.w.n]

ALso do nounwind

[nikic]

LGTM

llvm/lib/Transforms/Utils/InlineFunction.cpp

if (const Instruction *I = dyn_cast<Instruction>(VMI->first)) {
         NI->setMetadata(
             LLVMContext::MD_alias_scope,
             MDNode::concatenate(NI->getMetadata(LLVMContext::MD_alias_scope),
                                 MDNode::get(CalledFunc->getContext(), Scopes)));
     }
   }
 }
 
-static bool MayContainThrowingOrExitingCall(Instruction *Begin,
-                                            Instruction *End) {
+static bool MayContainThrowingOrExitingCallAfterCB(CallBase *Begin,
+                                                   ReturnInst *End) {
 
   assert(Begin->getParent() == End->getParent() &&
          "Expected to be in same basic block!");
+  auto BeginIt = Begin->getIterator();
+  assert(BeginIt != End->getIterator() && "Non-empty BB has empty iterator");
   return !llvm::isGuaranteedToTransferExecutionToSuccessor(
-      Begin->getIterator(), End->getIterator(), InlinerAttributeWindow + 1);
+      ++BeginIt, End->getIterator(), InlinerAttributeWindow + 1);
 }
 
 static AttrBuilder IdentifyValidAttributes(CallBase &CB) {
   AttrBuilder Valid(CB.getContext());
   // Only allow these white listed attributes to be propagated back to the
   // callee. This is because other attributes may only be valid on the call
   // itself, i.e. attributes such as signext and zeroext.
   if (auto DerefBytes = CB.getRetDereferenceableBytes())
for (auto &BB : *CalledFunction) {
     // }
     // caller() {
     //   %val = call nonnull @callee()
     // }
     // Here we cannot add the nonnull attribute on either foo or bar. So, we
     // limit the check to both RetVal and RI are in the same basic block and
     // there are no throwing/exiting instructions between these instructions.
     if (RI->getParent() != RetVal->getParent() ||
-        MayContainThrowingOrExitingCall(RetVal, RI))
+        MayContainThrowingOrExitingCallAfterCB(RetVal, RI))
       continue;
     // Add to the existing attributes of NewRetVal, i.e. the cloned call
     // instruction.
     // NB! When we have the same attribute already existing on NewRetVal, but
     // with a differing value, the AttributeList's merge API honours the already
     // existing attribute value (i.e. attributes such as dereferenceable,
     // dereferenceable_or_null etc). See AttrBuilder::merge for more details.
     AttributeList AL = NewRetVal->getAttributes();
     AttributeList NewAL = AL.addRetAttributes(Context, Valid);

llvm/test/Transforms/Inline/nonnull.ll

 ; CHECK-NEXT:    ret ptr [[R]]
 ;
   %r = call ptr @buz()
   ret ptr %r
 }
 
 define ptr @caller8() {
 ; CHECK-LABEL: define ptr @caller8() {
-; CHECK-NEXT:    [[R_I:%.*]] = call ptr @buz()
+; CHECK-NEXT:    [[R_I:%.*]] = call nonnull ptr @buz()
 ; CHECK-NEXT:    ret ptr [[R_I]]
 ;
   %r = call nonnull ptr @callee8()
   ret ptr %r
 }
 
 define ptr @callee9() {
 ; CHECK-LABEL: define ptr @callee9() {