PhilippvK commented on pull request #9243:
URL: https://github.com/apache/tvm/pull/9243#issuecomment-961777427
Sorry for not following up on this. While the CI bugs have been fixed in the
meantime, there are still 3 points open for discussion:
1. Currently the function `ModuleNode::GetFormat` is defined in
`src/runtime/module.cc` but each inherited class need to implement the trivial
method by itself. The reason for this decision is, that not every module is
supposed to have a format. I first thought only `CSourceModuleNode` and
`CSourceCrtMetadataModuleNode` needs to be updated but it seems like runtime
modules such as `CMSISNNModuleNode` and `EthosUModuleNode` are directly based
on 'runtime::ModuleNode' instead of 'CSourceModuleNode' leading to a need to
implement a 'GetFormat' function for them as well. Do you have an advice on how
to avoid this?
2. Coming up with proper unit tests for this feature: I think there are two
parts which shall be tested individually:
- The handling of the `format` argument in the
`runtime.CSourceModuleCreate` function invoced by severel BYOC kernels as well
as the 'GetFormat()' function which should return the specified value.
- Export of C++ kernels with the required file extension i.e. via the
Model Library Format. This should also include tests ensuring that special
cases (e.g. `nvcc` which has the `.cu` extension hardcoded) do not break.
3. As mentioned above, i was wondering, why the
`CSourceCrtMetadataModuleNode` is using the format `cc` instead of `c`, which
would result in a change of the file extension for this source file to
`tvmgen_default_lib0.cc` instead of `tvmgen_default_lib0.c` and therefore
likely break some Makefile-Setups where kernel filenames are hardcoded.
--
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]
