[PATCH] D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods.

[Manman Ren via Phabricator via cfe-commits]

manmanren added a comment.



In D86049#2255738 <https://reviews.llvm.org/D86049#2255738>, @rjmccall wrote:

> We just talk about it.  I agree with Nathan that we shouldn't just add this as a short-term hack; we should design the ABI right and then do what we want.
>
> I think these are basically all the ABI questions:
>
> - Is there a good reason to preserve signature compatibility with normal ObjC methods, or should we just drop the selector argument on direct methods?
> - Should we do the null test on the caller side or the callee side?
> - In direct class methods, is the caller's responsibility or the callee's to ensure that `self` is initialized?
>
> For the first, I think dropping the argument is fine.

Dropping the argument sounds good to me! We are already passing the argument as undefined.

> For the second, I'm less certain.

It feels cleaner to do the null test on the callee side (i.e inside the wrapper). It may have binary size advantage compared to checking at each callsite.

> For the third, making it the callee's responsibility is generally better for code size but harder to optimize.  But we only have the code-size cost on the caller side when materializing from a global, and that's actually something we can also eliminate as redundant.  So maybe the best approach is to introduce a weak+hidden thunk that we use when making a call to a direct class method on a specific class, and we have that thunk do the entire materialization sequence.

Completely agree. For direct class method, making sure 'self' is initialized on the callee side is cleaner and better for code size. But caller has the context to know whether the class is already initialized.
Maybe we can start with doing it on callee's side and make an incremental improvement by implementing the approach that can get both benefits.

Do we need to support direct methods with variadic arguments for the wrapper implementation or are we going with the non-wrapper version? @plotfi

Thanks!
Manman


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86049