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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
