Lunderberg commented on code in PR #13569:
URL: https://github.com/apache/tvm/pull/13569#discussion_r1092259667
##########
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:
> as call_extern implies call into a function with C calling convention
To double-check, is this the case for functions that exist within the same
module? That is, because the name resolution in the LLVM codegen
[preferentially resolves to
names](https://github.com/apache/tvm/blob/main/src/target/llvm/codegen_llvm.cc#L973)
within the existing module, my mental model was that the calling convention
was only fixed for calls into external functions, or for callees that may be
called externally. Therefore, the internal-only function would be sufficient
to modify any matching `call_extern` instances.
> So if we want to state that this is a convention that will be further
lowered, just a strawman: (e.g. InternalModuleCall, call_internal)
From the earlier design discussions, this was brought up as a potential
later step, for when using `call_extern` becomes too unwieldy, or unable to
express the degrees of freedom required by internal calls. Since it seemed
that the changes to `call_extern` were legal with the current semantics, we
hadn't moved to make a new builtin for it.
> IntraModule might be slightly too generic(since we can also use Packed
call intra module),
I'm not quite seeing the issue with the name here. The semantics we were
picturing was that while any calling convention may be used for internal calls,
only the `kIntraModule` calling convention That is, `kIntraModule` implies an
internal function, but not being `kIntraMoudule` does not imply the existence
of external calls. 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.
> After reading the context, seems we lean towards later. In that case, it
might still be useful to specify what would be the default lowering of
InternalModuleCall and call_internal.
The concern was that by specifying that behavior, it would enable
assumptions of the final calling convention before it had been selected, mixing
the different layers of abstraction. That is, somebody could erroneously
generate a callsite that follows the PackedFunc API, on the assumption that the
callee will also be lowered to the PackedFunc API. By leaving it unspecified,
it makes that misunderstanding less likely.
--
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]
