[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:1561
       for (Instruction &I : BB)
         if (CallInst *CI = dyn_cast<CallInst>(&I))
           CI->removeFnAttr(Attribute::NoCallback);
----------------
arsenm wrote:
> Drive by comment, but is this brokenly not checking for CallBase?
I'm not familiary enough with this code / LLVM to answer this. cc @gulfem who added this in D137360.

Anyway, marking this comment as "done" because if this needs to be fixed, it shouldn't be part of this change.


================
Comment at: llvm/lib/Linker/IRMover.cpp:1577-1579
+  if (DstF && SrcF && DstF->isDeclaration() && !SrcF->isDeclaration()) {
+    assert(DstF->arg_size() == SrcF->arg_size() &&
+           "Dst and Src should have the same signature.");
----------------
arsenm wrote:
> I thought we casted the function type at callsites for different signatures in different modules, so this wouldn't hold?
@tejohnson do you have any input on this?

(I last looked at this code in-depth in early December, but don't remember seeing anything relevant. `ninja check-all` is also clean and in a brief test `IRMover` would happily import a function with a different signature without creating any bitcasts).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139209