You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/04/11 17:49:44 UTC
[GitHub] [tvm] areusch commented on a diff in pull request #10189: [USMP] Adding support for U1 usecase for constant pools
areusch commented on code in PR #10189:
URL: https://github.com/apache/tvm/pull/10189#discussion_r847579183
##########
include/tvm/ir/memory_pools.h:
##########
@@ -60,10 +59,12 @@ struct PoolInfoNode : public Object {
*/
bool is_internal = false;
+ /*! \brief The targets linked to the pool */
+ Array<Target> targets;
Review Comment:
just curious why rw/ro went away?
##########
src/relay/backend/aot_executor_codegen.cc:
##########
@@ -744,13 +744,11 @@ class AOTExecutorCodegen : public MixedModeVisitor {
}
/*!
- * brief Calculate workspace sizes for PrimFuncs in the IRModule
+ * \brief Calculate workspace sizes for PrimFuncs in the IRModule
*/
Map<String, FunctionInfo> CalculateWorkspaceSizes(
const IRModule& lowered_mod, const Map<String, FunctionInfo>& function_metadata) {
- Executor executor_config = lowered_mod->GetAttr<Executor>(tvm::attr::kExecutor).value();
- Integer workspace_byte_alignment =
- executor_config->GetAttr<Integer>("workspace-byte-alignment").value_or(16);
+ Integer workspace_byte_alignment = GetModuleWorkspaceAlignment(lowered_mod);
Review Comment:
nit: make the name agree with the property, here and below
##########
src/relay/backend/executor.cc:
##########
@@ -91,7 +91,8 @@ TVM_REGISTER_EXECUTOR("aot")
.add_attr_option<Bool>("link-params", Bool(true))
.add_attr_option<Bool>("unpacked-api")
.add_attr_option<String>("interface-api")
- .add_attr_option<Integer>("workspace-byte-alignment");
+ .add_attr_option<Integer>("workspace-alignment")
Review Comment:
i still kind of like having a unit here...just curious what's the motivation for dropping it?
##########
tests/python/relay/aot/test_cpp_aot.py:
##########
@@ -117,8 +117,8 @@ def @main(%data : Tensor[(1, 3, 64, 64), uint8], %weight : Tensor[(3, 3, 5, 5),
mod = tvm.relay.build(
ir_mod,
params=params,
- target="c",
- executor=backend.Executor("aot", {"interface-api": "packed"}),
+ target="c --link-params=1",
Review Comment:
i think you shouldn't need `--link-params=1` if it's in the Executor, right?
##########
python/tvm/ir/memory_pools.py:
##########
@@ -112,15 +112,155 @@ def __init__(
)
+@register_object("ir.PoolInfoProperties")
+class PoolInfoProperties(Object):
+ """PoolInfo object holds information related to memory pools
+ where the statically sized allocate nodes will pooled into.
+
+ Parameters
+ ----------
+ size_hint_bytes : Optional[int]
+ The expected size hint to be used by the allocator.
+ The default value would be -1 which means the pool
+ is not size restricted.
+
+ clock_frequency_hz : Optional[int]
+ The clock frequency that the memory pool runs at in Hz.
+ If not specified/known, this will default to -1 indicating
+ it hasn't been defined.
+
+ read_bandwidth_bytes_per_cycle : Optional[int]
+ The read bandwidth of the memory pool in bytes/cycle.
+ If not specified/known, this will default to -1 indicating
+ it hasn't been defined.
+
+ write_bandwidth_bytes_per_cycle : Optional[int]
+ The write bandwidth of the memory pool in bytes/cycle.
+ If not specified/known, this will default to -1 indicating
+ it hasn't been defined.
+
+ read_latency_cycles : Optional[int]
+ The read latency of the memory pool in cycles.
+ If not specified/known, this will default to 0.
+
+ write_latency_cycles : Optional[int]
+ The write latency of the memory pool in cycles.
+ If not specified/known, this will default to 0.
+
+ target_burst_bytes : Optional[Union[Dict[Target, int], None]]
+ The burst length of the memory pool in bytes per target.
+ If not specified/known for a given target, a burst length
+ of 1 byte will be assumed.
+
+ """
+
+ def __init__(
+ self,
+ size_hint_bytes: Optional[int] = -1,
+ clock_frequency_hz: Optional[int] = -1,
+ read_bandwidth_bytes_per_cycle: Optional[int] = -1,
+ write_bandwidth_bytes_per_cycle: Optional[int] = -1,
+ read_latency_cycles: Optional[int] = 0,
+ write_latency_cycles: Optional[int] = 0,
+ target_burst_bytes=None, # Optional[Union[Dict[target.Target, int], None]]
+ ):
+ if not target_burst_bytes:
+ target_burst_bytes = dict()
+
+ self.__init_handle_by_constructor__(
+ _ffi_api.PoolInfoProperties, # type: ignore # pylint: disable=no-member
+ size_hint_bytes,
+ clock_frequency_hz,
+ read_bandwidth_bytes_per_cycle,
+ write_bandwidth_bytes_per_cycle,
+ read_latency_cycles,
+ write_latency_cycles,
+ target_burst_bytes,
+ )
+
+
+@register_object("ir.WorkspacePoolInfo")
+class WorkspacePoolInfo(PoolInfo):
+ """WorkspacePoolInfo object holds information related to RW memory pools
+ where the statically sized allocate nodes will pooled into.
+
+ Parameters
+ ----------
+ pool_name : str
+ The name of the memory pool
+
+ targets : list[Target]
+ A list of targets which could access the pool
+
+ pool_info_properties : PoolInfoProperties
+ The properties of the pool.
+ """
+
+ # pylint: disable=W0231
+ def __init__(
+ self,
+ pool_name: str,
+ targets, # Dict[Target, str]
+ pool_info_properties=None,
+ ):
+ if pool_info_properties is None:
+ pool_info_properties = PoolInfoProperties()
+
+ self.__init_handle_by_constructor__(
+ _ffi_api.WorkspacePoolInfo, # type: ignore # pylint: disable=no-member
+ pool_name,
+ targets,
+ pool_info_properties,
+ )
+
+
+@register_object("ir.ConstantPoolInfo")
+class ConstantPoolInfo(PoolInfo):
+ """ConstantPoolInfo object holds information related to RO memory pools
+ where the statically sized allocate nodes will pooled into.
+
+ Parameters
+ ----------
+ pool_name : str
+ The name of the memory pool
+
+ targets : list[Target]
+ describes which targets could access the pool
+
+ pool_info_properties : PoolInfoProperties
+ The properties of the pool.
+ """
+
+ # pylint: disable=W0231
+ def __init__(
+ self,
+ pool_name: str,
+ targets, # list[Target]
Review Comment:
i think could use Optional[list[Target]] or Optional[list["Target"]]
##########
tests/python/relay/aot/test_crt_aot_usmp.py:
##########
@@ -58,22 +58,72 @@ def check_for_no_tvm_backendallocworkspace_calls(mod: tvm.runtime.module):
) == 0, "This is failing because USMP was unable to plan for every tir.allocate node"
+# U1 test case
+@parametrize_aot_options
+def test_synthetic(interface_api, use_unpacked_api, test_runner):
+ mod, params = tvm.relay.testing.synthetic.get_workload()
+ main_func = mod["main"]
+ shape_dict = {p.name_hint: p.checked_type.concrete_shape for p in main_func.params}
+ type_dict = {p.name_hint: p.checked_type.dtype for p in main_func.params}
+
+ input_data = np.ones(shape_dict["data"]).astype(type_dict["data"])
+ params = {}
+ for name, shape in shape_dict.items():
+ if name != "data":
+ params[name] = np.ones(shape_dict[name]).astype(type_dict[name])
+
+ inputs = {"data": input_data}
+ output_list = generate_ref_data(mod, inputs, params)
+ config = (
+ {
+ "tir.disable_vectorize": True,
+ "tir.disable_storage_rewrite": True,
+ "tir.usmp.enable": True,
+ "tir.usmp.algorithm": "greedy_by_conflicts",
+ },
+ )
+
+ pass_config = {"tir.usmp.enable": True}
+ test_runner = AOTTestRunner(
+ makefile=test_runner.makefile,
+ prologue=test_runner.prologue,
+ epilogue=test_runner.epilogue,
+ includes=test_runner.includes,
+ parameters=test_runner.parameters,
+ pass_config={**test_runner.pass_config},
+ )
+ test_runner.pass_config.update(*config)
+ compile_and_run(
+ AOTTestModel(module=mod, inputs=inputs, outputs=output_list, params=params),
+ test_runner,
+ interface_api,
+ use_unpacked_api,
+ )
+
+
@pytest.mark.parametrize(
- "workspace_byte_alignment,main_workspace_size",
+ "workspace_byte_alignment,constant_byte_alignment,main_workspace_size",
[
- (8, 17280),
- (16, 17280),
- (256, 17792),
+ (8, 8, 18228),
+ (16, 8, 18228),
+ (256, 8, 18740),
+ (8, 16, 18236),
+ (16, 16, 18236),
+ (256, 16, 18748),
+ (8, 256, 19084),
+ (16, 256, 19084),
+ (256, 256, 19596),
],
)
-def test_memory_planning(workspace_byte_alignment, main_workspace_size):
+def test_memory_planning(workspace_byte_alignment, constant_byte_alignment, main_workspace_size):
Review Comment:
nit: align names with Target spec
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org