[PATCH] D105733: [OpaquePtr] Require matching signature in getCalledFunction()

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

dexonsmith added a comment.

In D105733#2872394 <https://reviews.llvm.org/D105733#2872394>, @nikic wrote:

> "Call to bitcasted function" is considered non-canonical in LLVM IR and InstCombine will try to move casts from the call to the arguments (see transformConstExprCastCall). I believe it should succeed for any cases where opaque pointers will make the signatures the same. Thanks to this, most passes will only see the canonicalized form that performs a direct call. For that reason, I wouldn't expect this to have a major impact during the opaque pointer switch, at least relative to other changes (much less bitcasts, more i8 GEPs, etc.)

Ah, thanks for explaining. I agree that makes it lower value to front-load.

In D105733#2878223 <https://reviews.llvm.org/D105733#2878223>, @nikic wrote:

> In D105733#2872599 <https://reviews.llvm.org/D105733#2872599>, @aeubanks wrote:
>
>> intrinsics should always have matching arguments/signatures, we should perhaps have a verifier check for that?
>
> That sounds reasonable to me. Intrinsic calls are always required to be direct, so verifying the signature should be safe and will help prevent mistakes. Implemented in D106013 <https://reviews.llvm.org/D106013>.

As per https://reviews.llvm.org/D106013#2878472, I wonder how hard it would be in practice to maintain this invariant for all functions (not just intrinsics), making this patch unnecessary. Besides an update to `ValueMapper::remapInstruction` to fix the inliner, what else is required?


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

https://reviews.llvm.org/D105733