[llvm] [Inliner] Fix bug where attributes are propagated incorrectly (PR #109347)

[via llvm-commits]

================
@@ -1477,6 +1483,12 @@ static void AddReturnAttributes(CallBase &CB, ValueToValueMapTy &VMap) {
     auto *NewRetVal = dyn_cast_or_null<CallBase>(VMap.lookup(RetVal));
     if (!NewRetVal)
       continue;
+
+    // The RetVal might have be simplified during the inlining
+    // process. Only propagate return attributes if we are in fact calling the
+    // same function.
+    if (RetVal->getCalledFunction() != NewRetVal->getCalledFunction())
----------------
goldsteinn wrote:

> This doesn't seem like it will reliably detect the issue.
> 
> First off, if both calls are indirect, both calls to getCalledFunction() would return nullptr, in which case you aren't getting any useful information at all.
> 

Fair, I think its fine to just disable all prop for indirect functions.

> Second, even if both calls are calling the "same" function, it might not have the same behavior if the arguments are different. Or global state is different. Not sure how likely this is given the limited simplifications the inline performs, but it's hard to rule out.
> 
> So I think we need to avoid using VMap, and pass down the equivalence information some other way.

Is that really a valid concern? We don't allow arbitrary transforms on the new instructions. Within the confines of what we do do, I don't see how we could change global state or produce and argument that is not semantically equiv to what was in the original.

That being said, we could also move the propagation code to the function cloning logic before simplifications/etc.

https://github.com/llvm/llvm-project/pull/109347