[PATCH] D106013: [Verifier] Require same signature for intrinsic calls

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added a comment.

In D106013#2878243 <https://reviews.llvm.org/D106013#2878243>, @nikic wrote:

> In D106013#2878226 <https://reviews.llvm.org/D106013#2878226>, @dexonsmith wrote:
>
>>> We probably can't enforce this in general due to unreachable (possibly de-indirected) calls, but I don't see a reason why we can't enforce this for intrinsics at least.
>>
>> Can you explain in more detail why the verifier couldn't check this in unreachable (possibly de-indirected) calls? Why can't we require transforms to be type-correct here (by calling RAUW with a bitcast, or similar)?
>
> The same reasoning as https://llvm.org/docs/FAQ.html#why-does-instcombine-simplifycfg-turn-a-call-to-a-function-with-a-mismatched-calling-convention-into-unreachable-why-not-make-the-verifier-reject-it applies here, or at least I would assume so.

Thanks for the link. The argument there is that we don't want all transforms to have to worry about calling conventions. (I guess we can't take the address of intrinsics, and that's why this verifier check is okay for them?)

We could choose to make a different decision for type-correctness of function calls.

My intuition is that it's easier for type-correctness than for calling conventions.

- Any code replacing one function with another is already calling RAUW with a bitcast to the new function type. (Doesn't catch calling conventions of course, but maintains FunctionType correctness.)
- Any existing code creating a new CallInst should similarly be creating the expected bitcasts.
- The inliner would need a patch. I spent some time looking at the code. The hard part (probably obvious to you) is that there won't be bitcasts at the call site, which means `ValueMapper::mapValue()` can't return the "right" result. You'd need to catch this up a level, in `ValueMapper::remapInstruction`, by checking for `CallBase` and adding a bitcast if there's a type mismatch. There's related code already in that function for when there's a TypeMapper, added in 348de69a3045e017d4d614213d28e69a6fdd4b73 to allow indirect calls to round-trip through bitcode and textual IR.

Are there other cases? I wonder if they're enumerable, or if it'll be a long tail... my intuition is the former, but I don't really know.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106013