lhutton1 commented on code in PR #10189:
URL: https://github.com/apache/tvm/pull/10189#discussion_r897730969
##########
include/tvm/runtime/metadata_types.h:
##########
@@ -82,6 +87,21 @@ struct TVMTensorInfo {
DLDataType dtype;
};
+/*!
+ * \brief Describes one constant argument to `run_model`.
+ *
+ */
+struct TVMConstantInfo {
+ /*! \brief Name of the constant */
+ const char* name_hint;
+ /*! \brief Offset in bytes of the constant */
+ int64_t byte_offset;
+ /*! \brief lenght of the data_bytes field */
Review Comment:
Nit: Length
##########
tests/python/unittest/test_tir_usmp_analysis_extract_bufferinfo.py:
##########
@@ -1365,9 +1362,9 @@ def run_model(data: T.handle, output: T.handle) -> None:
def test_multiple_calls_to_same_primfunc():
Review Comment:
Was this added? I couldn't find it after a naive look
##########
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.
Review Comment:
Nit: ...are pooled into.
##########
src/tir/usmp/transform/convert_pool_allocations_to_offsets.cc:
##########
@@ -53,14 +54,20 @@ class PoolAllocationToOffsetConverter : public
StmtExprMutator {
: pool_allocations_(pool_allocations),
emit_tvmscript_printable_(emit_tvmscript_printable) {
module_ = module->ShallowCopy();
for (const auto& kv : pool_allocations) {
- // TODO(@manupa-arm): add AllocateConstNode when it is available
- ICHECK(kv.first->IsInstance<AllocateNode>());
- Allocate allocate_node = Downcast<Allocate>(kv.first);
+ size_t extent_size = -1;
+ if (kv.first->IsInstance<AllocateNode>()) {
+ Allocate allocate_node = Downcast<Allocate>(kv.first);
+ extent_size = CalculateExtentsSize(allocate_node.operator->());
+ } else if (kv.first->IsInstance<AllocateConstNode>()) {
+ AllocateConst allocate_const_node = Downcast<AllocateConst>(kv.first);
+ extent_size = CalculateExtentsSize(allocate_const_node.operator->());
+ } else {
+ LOG(FATAL) << "Not supported";
Review Comment:
Think we missed this one
##########
src/target/llvm/codegen_cpu.cc:
##########
@@ -1104,8 +1103,14 @@ class MetadataSerializerLLVM : public AttrVisitor {
llvm::ConstantInt::get(llvm_types_->t_uint8, value->lanes(), false /*
isSigned */)}));
}
+ // Serialiding NDArray as tuple of len, data
Review Comment:
Nit: Serializing
##########
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:
Curious about why this would be? It seems we already have List and Optional
imported above. For consistency, type hints would be better if we can use them
##########
python/tvm/relay/build_module.py:
##########
@@ -631,7 +652,7 @@ def _make_executor(self, expr=None):
# generated code.
temp_so_dir = contrib_utils.TempDirectory()
temp_so = temp_so_dir / "temp.so"
- mod.export_library(temp_so, cc="gcc", options=["-std=c11"])
+ mod.export_library(temp_so, cc="c++", options=["-std=gnu++14"])
Review Comment:
Looks as though its currently -std=c++11, should this be -std=c11?
##########
src/target/source/source_module.cc:
##########
@@ -283,6 +291,55 @@ class CSourceCrtMetadataModuleNode : public
runtime::ModuleNode {
code_ << "}\n\n";
}
+ void GenerateConstantBuffer(const ConstantPoolInfoNode* pool_info, size_t
allocated_size) {
+ size_t offset = 0;
+ if (pool_info->constant_info_array.size() > 0) {
+ // Pool is RO, form an initialized struct
+ code_ << "__attribute__((section(\".rodata.tvm\"), ";
+ code_ << "))\n";
+ code_ << "static struct " << pool_info->pool_name << " {\n";
+ // emit struct field names
+ std::vector<ConstantInfo>
const_info_vec(pool_info->constant_info_array.begin(),
+
pool_info->constant_info_array.end());
+ std::sort(const_info_vec.begin(), const_info_vec.end(),
+ [](const ConstantInfo& a, const ConstantInfo& b) {
+ return a->byte_offset->value < b->byte_offset->value;
+ });
+ for (const auto& const_info : const_info_vec) {
+ const auto& data = const_info->data;
+ const auto& offs = const_info->byte_offset;
+ int64_t num_elements = std::accumulate(data.Shape().begin(),
data.Shape().end(), 1,
+ std::multiplies<int64_t>());
+ code_ << " ";
+ codegen_c_base_.PrintType(data.DataType(), code_);
+ code_ << " " << const_info->name_hint << "[" << num_elements
+ << "] __attribute__((packed, aligned(" <<
metadata_->constant_alignment << ")));";
+ code_ << " // " << num_elements * data.DataType().bytes()
+ << " bytes, aligned offset: " << offs << "\n";
+ }
+ code_ << "} " << pool_info->pool_name << " = {\n";
+
+ // emit struct field initialization data
+ for (const auto& const_info : const_info_vec) {
+ code_ << " ." << const_info->name_hint << " = {\n";
+ codegen::NDArrayDataToC(const_info->data, 4, code_);
+ code_ << " },\n";
+ }
+ code_ << "};";
+ code_ << "// of total size " << allocated_size << " bytes, aligned: " <<
offset << " bytes\n";
+ } else {
+ LOG(FATAL) << "No constant data in constant pool found "
+ << PrettyPrint(GetRef<ObjectRef>(pool_info));
+ }
+ }
+
+ void GenerateWorkspaceBuffer(const WorkspacePoolInfoNode* pool_info, size_t
allocated_size) {
+ code_ << "__attribute__((section(\".bss.noinit.tvm\"), ";
Review Comment:
We could probably take this in a follow up as it seems not to affect
functionality?
##########
include/tvm/tir/usmp/utils.h:
##########
@@ -44,6 +44,126 @@ constexpr const char* kUSMPAlgorithmOption =
"tir.usmp.algorithm";
namespace tir {
namespace usmp {
+/*
+ * \brief The ConstantInfoNode contains numeric literal in RO pool
+ */
+struct ConstantInfoNode : public Object {
Review Comment:
Did this one get done?
##########
tests/python/relay/aot/test_cpp_aot.py:
##########
@@ -138,7 +140,7 @@ def test_mobilenet(enable_usmp, target_kind):
temp_dir = tvm.contrib.utils.TempDirectory()
test_so_path = temp_dir / "test.so"
- mod.export_library(test_so_path, cc="gcc", options=["-std=c11"])
+ mod.export_library(test_so_path, cc="c++", options=["-std=gnu++14", "-g3",
"-O0"])
Review Comment:
should this also be similar to the above change?
`mod.export_library(test_so_path, cc="gcc", options=["-std=c++11", "-g3",
"-O0"])`
--
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]
