[PATCH] D139209: [IRMover] Remove UB implying parameter attributes when necessary

[Tim Neumann via Phabricator via llvm-commits]

TimNN marked an inline comment as done.
TimNN added inline comments.


================
Comment at: llvm/lib/Linker/IRMover.cpp:608
+  // The known incompatibilities only apply if `Dst` is a declaration and `Src`
+  // the corresponding definition, ignore other cases.
+  if (Src.isDeclaration() || !Dst.isDeclaration())
----------------
tejohnson wrote:
> I thought this could only happen if Src's definition is not linked in.
> 
> Ah, Src may contain a def but we may not link in that def (depending on whether we decide to invoke linkGlobalValueBody). I believe if the def of Src was linked in, we wouldn't have an issue, since presumably that def would contain the correct attributes? I assume if you remove the "noinline" from inner in the Input file for the test it's def gets linked in - I forget, does that work correctly?
> 
> Assuming the answer to the last question above is yes, then I think you would only want to call this when we don't invoke linkGlobalValueBody.
> 
> Then I don't think you need or want this if statement at all (we've already checked in the caller that dest is a declaration).
Ack, makes sense, I think I understand this code a bit better now. Removed this `if` statement and change the code to only call `ensureFunctionCompatibility` if the body did not get linked.

And yes, removing the `noinline` fixes the problem (though the original issues happened without `noinline`, i.e. the optimizier decided to not inline `@inner` for some other reason).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139209