tqchen commented on code in PR #13569:
URL: https://github.com/apache/tvm/pull/13569#discussion_r1092462583
##########
include/tvm/ir/function.h:
##########
@@ -63,6 +63,43 @@ enum class CallingConv : int {
* - Implementation: defined by device runtime(e.g. runtime/cuda)
*/
kDeviceKernelLaunch = 2,
+ /*!
+ * \brief For functions only called by other functions within the same IR
module.
+ *
+ * This calling convention exists to support one PrimFunc calling another
PrimFunc
+ * within the same IRModule.
+ *
+ * Overview / Purpose:
+ *
+ * - This calling convention is intended only for PrimFuncs whose callers
reside in
+ * the same IRModule. This is the only supported use case.
+ *
+ * - The details of the calling convention may change frequently as TVM
evolves.
+ * Therefore users are discouraged from attempting to use this calling
convention
+ * outside of the supported use case(s).
+ *
+ * Current mechanics / usage requirements:
+ *
+ * - A PrimFunc with this calling convention will NOT undergo any of the
signature
+ * transformations provided by the MakePackedAPI pass.
+ *
+ * - Supported use cases, and their corresponding unit tests, are all
expressed as
+ * TVMScript.
+ *
+ * - The callsite must use `T.call_extern`.
Review Comment:
Thanks for the discussion.
> The difference would be that a pass could modify the signature of a
kIntraModule function (e.g. removing an unused argument from the signature,
simultaneously updating all callsites), while a PackedFunc could not be
modified without breaking the potential external callsites.
This have to do with linkage which is somewhat parallel to the calling
convention. We should be able to modify a signature of a function if it is not
being referred from outside (in our current case, no `global_symbol` attribute
indicates a private linkage), and we can do such setups independent from the
calling convention.
> it would enable assumptions of the final calling convention before it had
been selected, mixing the different layers of abstraction
Indeed this is what we aim to avoid here. That is why normally we try to
have pairs of(callsite intrinsic, and callee side calling convention
annotations). For example,
- `InternalModuleCall` (callee side conv) and `call_internal`(caller side
intrinsic) would make such pairing clear.
In the current proposal, we are trying to pair (`kIntraModule` and
`call_extern`). Where previously in our convention `call_extern` was normally
paired with C calling conv(mainly because we also can generate such calls).
BTW, I agree that it is OK to leave default lowering behavior unspecified,
as long as the pairing is clear. But in this case, the possible confusion is
that we already have lowering behavior for `call_extern` defined by our
backend. So only leaving the callee site unspecified could leads to possible
point of confusion.
So the main comment here is let us make sure our pairing is clear. Ideally
we have a unique set of pairings, e.g. `call_extern` only maps to one
callee-site calling conv. Introducing a extra intrinsic to pair with this one,
or clarify that this is the only thing that call_extern pairs(in which case
perhaps C calling conv is more proper) might be more clear.
The internal property have things to do with linkage and can be handled
separately
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
