echuraev commented on code in PR #13349:
URL: https://github.com/apache/tvm/pull/13349#discussion_r1032425689
##########
include/tvm/driver/driver_api.h:
##########
@@ -78,20 +78,22 @@ TVM_DLL transform::Sequential
HostModulePassManager(IRModule mixed_mod, Target t
* \brief Lower an IRModule (optimize with it with the pass list defined in
CreatePassList)
* \param mod The IRmodule to lower
* \param simple_mode Disables the loop partition pass. Defaults to false.
+ * \param target The device target to provide additional characteristics.
Defaults to not defined.
* \return The result module.
*/
-TVM_DLL IRModule LowerModule(IRModule mod, bool simple_mode = false);
+TVM_DLL IRModule LowerModule(IRModule mod, bool simple_mode = false, Target
target = {});
Review Comment:
I have some doubts about introducing the new parameter with target to many
lowering functions. As far as I understood, the only one thing why we need to
have this `target` parameter is for passing value of `vtcm_capacity` to the
lowering functions.
From my point of view, we shouldn't modify this common API. As for me, the
better way is to pass this option through `PassContext` (e.g. in the same way
it works for `relay.ToMixedPrecision.keep_orig_output_dtype`).
@tqchen, @csullivan, @junrushao, @tkonolige, In file
[test_vtcm.py](https://github.com/apache/tvm/pull/13349/files#diff-9380a067fbc7e640cb0257906fadaf5c400f07f46500d530af7c601b29855eb6R83-R95)
@Icemist has presented possible ways for passing value of `vtcm_capacity`:
1. By passing target as a parameter to the `lower` function:
```python
_raises_exception(lambda: tvm.lower(sch.mod["main"], target=target)) ==
limited
```
2. By using `with` statement:
```python
with target:
assert (
_raises_exception(lambda: tvm.lower(sch.mod["main"])) == limited
), "VTCM memory allocation limiter does not work correctly "
```
3. By passing value of `vtcm_capacity` to `PassContext`:
```python
with tvm.transform.PassContext(config={"tir.vtcm_capacity":
vtcm_capacity}):
assert (
_raises_exception(lambda: tvm.lower(sch.mod["main"])) == limited
), "VTCM memory allocation limiter does not work correctly "
```
Personally, I prefer the third option, because in this case, I believe, it
is not necessary to modify the API of lowering functions. What do you think
about that?
--
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]
