You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by "vinx13 (via GitHub)" <gi...@apache.org> on 2024/07/09 23:03:20 UTC
[PR] [Relax] Implement R.ensure_aligned and update memory planning for R.view [tvm]
vinx13 opened a new pull request, #17145:
URL: https://github.com/apache/tvm/pull/17145
Previously, `R.view` was legalized to extern call to `runtime.TVMArrayCreateView` during `LegalizeOps`. This call to extern func can't be properly handled by `StaticBlockPlanMemory` because it assumes the extern func does not retain the input buffer. Extern func returning a view of the input would break the ref count of the buffer. This PR defers the legalization of `R.view` so that it can be explicitly handled by memory planning.
A new op `R.ensure_aligned` is added as discussed in #16955
cc @tqchen @yongwww @Lunderberg
--
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
Re: [PR] [Relax] Implement R.ensure_zero_offset and update memory planning for R.view [tvm]
Posted by "tqchen (via GitHub)" <gi...@apache.org>.
tqchen commented on PR #17145:
URL: https://github.com/apache/tvm/pull/17145#issuecomment-2271129320
cc @Lunderberg let us merge this in
--
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
Re: [PR] [Relax] Implement R.ensure_aligned and update memory planning for R.view [tvm]
Posted by "Lunderberg (via GitHub)" <gi...@apache.org>.
Lunderberg commented on PR #17145:
URL: https://github.com/apache/tvm/pull/17145#issuecomment-2223289279
> Perhaps one way to make it more clear is to rename LowerVMBuiltin to LowerRuntimeBuiltin, which have clear indication that that is a pass which takes charge of lowering all implementaitons of runtime builtin functions.
I like this idea, but I don't think we should move the definition of the legalized form into the `LowerRuntimeBuiltin`. What if we were to instead add a new attribute, which has the same signature as `FLegalize`, but would be applied at the later point. This would allow `LowerRuntimeBuiltin` to replace anything that has the `FLowerBuiltin` attribute, and wouldn't require a distinction between different levels of `FLegalize`.
That would also allow `FLowerBuiltin` to only run after `ToNonDataflow`, and to be implemented in terms of impure functions. By constrast, since `FLegalize` may replace a call within a dataflow block, the implementation cannot be in terms of an impure call.
> One can also envision in future we have a codegen approach to get a view function that get inlined which is not needed in the VM approach.
I like this idea, and have been toying around with some TIR implementations. The key limitation at the moment is the inability to construct and return a new `NDArray` if required. (Similar to the difficulties in returning a string that are blocking https://github.com/apache/tvm/pull/16836 and https://github.com/apache/tvm/pull/17103.)
--
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
Re: [PR] [Relax] Implement R.ensure_aligned and update memory planning for R.view [tvm]
Posted by "Lunderberg (via GitHub)" <gi...@apache.org>.
Lunderberg commented on PR #17145:
URL: https://github.com/apache/tvm/pull/17145#issuecomment-2221383953
> Based on the current grouping, seems quite a bit of the runtime function dispatchings happens in LowerBuiltin, while legalizeOps primarily focused on lowering to TIR related functions.
I don't think distinguishing between the style of implementation is a useful distinction to make. The important distinction is what functionality must still be observable outside of the operator, not the functional form of the legalized expression.
--
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
Re: [PR] [Relax] Implement R.ensure_aligned and update memory planning for R.view [tvm]
Posted by "Lunderberg (via GitHub)" <gi...@apache.org>.
Lunderberg commented on code in PR #17145:
URL: https://github.com/apache/tvm/pull/17145#discussion_r1679659483
##########
src/relax/transform/static_plan_block_memory.cc:
##########
@@ -286,8 +286,13 @@ class TokenAllocator1D {
std::vector<StorageToken> full_pool_;
};
-/*! \brief Check if the input op is "relax.reshape". */
-bool IsReshape(const Expr& op) { return op.same_as(Op::Get("relax.reshape")); }
+/*! \brief Check if the input op is a memory op that may return the same buffer. */
Review Comment:
Thank you on the updated docstring. As I'm looking at it, we may want to add this as another operator attribute (e.g. `.set_attr<Bool>("ReturnMayAliasArgument", Bool(true))`), but that could be a follow-up PR instead.
##########
include/tvm/runtime/device_api.h:
##########
@@ -240,6 +240,10 @@ class TVM_DLL DeviceAPI {
return device_type != kDLCPU && device_type != kDLMicroDev;
}
+ static bool SupportsPointerArithmetics(int device_type) {
Review Comment:
Since we already have a vtable for `DeviceAPI`, this should be a virtual function instead of a static boolean. That would allow individual `DeviceAPI` implementations to independently mark that they support the pointer-arithmetic. (It would also allow checking for driver-dependent support, such as vulkan support for the optional `VK_KHR_buffer_device_address` feature.)
Since host-side pointer arithmetic is not the default behavior for `DLTensor::data`, the default implementation in `DeviceAPI` would return false, and it could be overridden in `CPUDeviceAPI` and `CUDADeviceAPI` to return true.
##########
src/relax/op/memory/view.cc:
##########
@@ -58,9 +58,9 @@ StructInfo InferStructInfoView(const Call& call, const BlockBuilder& ctx) {
if (auto opt = sinfo.as<TensorStructInfo>()) {
return opt.value();
} else {
- LOG(FATAL) << "TypeError: "
- << "Operator " << call->op << " expects first argument to be a tensor, "
- << "but received " << arg_data << " with type " << sinfo;
+ LOG(FATAL) << "TypeError: " << "Operator " << call->op
Review Comment:
Nitpick: This looks like just a formatting change, but `clang-format` would produce the old formatting.
##########
src/runtime/relax_vm/builtin.cc:
##########
@@ -545,6 +545,23 @@ TVM_REGISTER_GLOBAL("vm.builtin.tensor_to_shape").set_body_typed([](NDArray data
return ShapeTuple(out_shape);
});
+TVM_REGISTER_GLOBAL("vm.builtin.ensure_zero_offset").set_body_typed([](NDArray data) {
+ if (data->byte_offset == 0) {
+ return data;
+ }
+ if (DeviceAPI::SupportsPointerArithmetics(data->device.device_type)) {
Review Comment:
Can we also restrict this to `data->byte_offset % kAllocAlignment == 0`? The alignment guarantee from TVM's allocators ensure that `NDArray->data % kAllocAlignment == 0`. This is checked when taking ownership of a `DLManagedTensor*`, and would raise an error in `NDArray::FromDLPack` if it fails.
##########
python/tvm/relax/transform/transform.py:
##########
@@ -586,14 +586,14 @@ def ComputePrimValue() -> tvm.ir.transform.Pass:
return _ffi_api.ComputePrimValue() # type: ignore
-def VMBuiltinLower() -> tvm.ir.transform.Pass:
+def LowerRuntimeBuiltin() -> tvm.ir.transform.Pass:
Review Comment:
I feel silly for this request after having pushed for the name change, but can we leave a backwards-compatibility wrapper for `VMBuiltinLower`? Otherwise, we would break existing external code (e.g. [here](https://github.com/mlc-ai/mlc-llm/blob/main/python/mlc_llm/compiler_pass/pipeline.py#L173) in MLC-LLM). I'm thinking something like the following:
```python
import warnings
def VMBuiltinLower():
warnings.warn(
"tvm.relax.transform.VMBuiltinLower has been renamed to 'LowerRuntimeBuiltin'. "
"This wrapper is for backwards compatibility, and will be removed in a later update."
)
return LowerRuntimeBuiltin()
```
That way, we have a period of time when either `VMBuiltinLower()` or `LowerRuntimeBuiltin()` can be used interchangeably. During this period, external code can update to the latest TVM without breakage, but will receive a warning whenever they use `VMBuiltinLower()`. Removing `VMBuiltinLower()` entirely after the grace period would then be much less painful.
##########
include/tvm/runtime/device_api.h:
##########
@@ -240,6 +240,10 @@ class TVM_DLL DeviceAPI {
return device_type != kDLCPU && device_type != kDLMicroDev;
}
+ static bool SupportsPointerArithmetics(int device_type) {
Review Comment:
Also, a nitpick: This isn't whether the device supports pointer arithmetic, but whether pointer arithmetic of a device-owned `void* DLTensor::data` may be performed on the host. The TVM backends for both Vulkan and OpenCL support pointer arithmetic, but only within the generated kernels. Neither support pointer arithmetic being performed on the host.
##########
tests/python/relax/test_op_view.py:
##########
@@ -483,18 +483,7 @@ def main(A: R.Tensor([4096], "float32")):
class Expected:
@R.function
def main(A: R.Tensor([4096], "float32")):
- B = R.ExternFunc(
- "runtime.TVMArrayCreateView",
- R.Callable(
- derive_func="tvm.relax.struct_info.infer_view_sinfo",
- purity=True,
- ),
- )(
- A,
- R.shape([64, 64]),
- R.dtype("float32"),
- R.prim_value(0),
- )
+ B = R.memory.view(A, shape=R.shape([64, 64]), dtype="float32", relative_byte_offset=0)
return B
After = tvm.relax.transform.LegalizeOps()(Before)
Review Comment:
After replacing the `.set_attr<FLegalize>` for `R.memory.view` with `.set_attr<FLowerBuiltin>`, the changes to these unit tests can be reverted. Instead, any use of `LegalizeOps` in the unit tests would instead call `LowerRuntimeBuiltin`.
##########
src/relax/op/memory/view.cc:
##########
@@ -334,13 +334,12 @@ Expr LegalizeView(const BlockBuilder& bb, const Call& call) {
relative_byte_offset = relax::PrimValue::Int64(0);
}
- StructInfoDeriveFunc infer_sinfo_env_func;
- infer_sinfo_env_func = EnvFunc::Get("tvm.relax.struct_info.infer_view_sinfo");
- auto runtime_view_sinfo = FuncStructInfo::OpaqueFunc(infer_sinfo_env_func, true);
-
- ExternFunc runtime_view_func("runtime.TVMArrayCreateView", runtime_view_sinfo);
+ if (shape.same_as(call->args[1]) && dtype.same_as(call->args[2]) &&
+ relative_byte_offset.same_as(call->args[3])) {
+ return call;
+ }
- return Call(runtime_view_func, {data, shape, dtype, relative_byte_offset});
+ return Call(call->op, {data, shape, dtype, relative_byte_offset});
Review Comment:
I don't think we want to do the inference of the shape/dtype prior to lowering, because it could result in unexpected StructInfo inference later on.
Suppose we have `R.memory.view(arg, shape=[16])`. This returns a view into the first 16 elements of `arg`, without changing the dtype. If an `IRModule` pass updates the datatype of `arg`, then that new datatype should also propagate to the view. However, legalizing it to `R.memory.view(arg, shape=[16], dtype="float16")` would return a view into `arg`, interpreting the first 32 bytes as if they were `"float16"`. Now, if an `IRModule` pass updates the datatype of `arg`, the view would still be `"float16"`. To avoid this issue, the unknown arguments shouldn't be filled in until the lowering is about to occur.
What if we were to remove `.set_attr<FLegalize>` altogether, and only have `.set_attr<FLowerBuiltin>`? That way, we preserve the `R.memory.view` as-is until it is ready to be lowered. The `LegalizeOps` pass would then be a no-op for `R.memory.view`, and only the `LowerRuntimeBuiltin` pass would change it at all.
--
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
Re: [PR] [Relax] Implement R.ensure_zero_offset and update memory planning for R.view [tvm]
Posted by "Lunderberg (via GitHub)" <gi...@apache.org>.
Lunderberg merged PR #17145:
URL: https://github.com/apache/tvm/pull/17145
--
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
Re: [PR] [Relax] Implement R.ensure_zero_offset and update memory planning for R.view [tvm]
Posted by "Lunderberg (via GitHub)" <gi...@apache.org>.
Lunderberg commented on code in PR #17145:
URL: https://github.com/apache/tvm/pull/17145#discussion_r1704138623
##########
python/tvm/relax/transform/__init__.py:
##########
@@ -84,14 +85,13 @@
function_pass,
)
+from .attach_external_modules import AttachExternModules
+from .fast_math import FastMathTransform
from .ipc_allreduce_rewrite import IPCAllReduceRewrite
from .lazy_transform_params import LazyTransformParams
from .lower_gpu_ipc_alloc_storage import LowerGPUIPCAllocStorage
from .optimize_layout_transform import OptimizeLayoutTransform
from .remove_redundant_reshape import RemoveRedundantReshape
-from .fast_math import FastMathTransform
-from .fuse_transpose_matmul import FuseTransposeMatmul
Review Comment:
Looks like the import of `FuseTransposeMatmul` was dropped when sorting the imports. The last failing unit test is caused by the dropped import.
--
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
Re: [PR] [Relax] Implement R.ensure_zero_offset and update memory planning for R.view [tvm]
Posted by "Lunderberg (via GitHub)" <gi...@apache.org>.
Lunderberg commented on PR #17145:
URL: https://github.com/apache/tvm/pull/17145#issuecomment-2263451826
@vinx13 I took a look at the current CI failures, and it looks like it pretty close to passing. If you'd like, applying the diff below should resolve the last 4 failing tests in CI.
[pr_17145_diff.txt](https://github.com/user-attachments/files/16459245/pr_17145_diff.txt)
--
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
Re: [PR] [Relax] Implement R.ensure_aligned and update memory planning for R.view [tvm]
Posted by "vinx13 (via GitHub)" <gi...@apache.org>.
vinx13 commented on code in PR #17145:
URL: https://github.com/apache/tvm/pull/17145#discussion_r1678444377
##########
src/relax/op/memory/view.cc:
##########
@@ -334,13 +334,12 @@ Expr LegalizeView(const BlockBuilder& bb, const Call& call) {
relative_byte_offset = relax::PrimValue::Int64(0);
}
- StructInfoDeriveFunc infer_sinfo_env_func;
- infer_sinfo_env_func = EnvFunc::Get("tvm.relax.struct_info.infer_view_sinfo");
- auto runtime_view_sinfo = FuncStructInfo::OpaqueFunc(infer_sinfo_env_func, true);
-
- ExternFunc runtime_view_func("runtime.TVMArrayCreateView", runtime_view_sinfo);
+ if (shape.same_as(call->args[1]) && dtype.same_as(call->args[2]) &&
+ relative_byte_offset.same_as(call->args[3])) {
+ return call;
+ }
- return Call(runtime_view_func, {data, shape, dtype, relative_byte_offset});
+ return Call(call->op, {data, shape, dtype, relative_byte_offset});
Review Comment:
This only does the inference of `void` type args and leave the lowering to the later pass.
--
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
Re: [PR] [Relax] Implement R.ensure_aligned and update memory planning for R.view [tvm]
Posted by "tqchen (via GitHub)" <gi...@apache.org>.
tqchen commented on PR #17145:
URL: https://github.com/apache/tvm/pull/17145#issuecomment-2223470289
> I like this idea, but I don't think we should move the definition of the legalized form into the LowerRuntimeBuiltin. What if we were to instead add a new attribute, which has the same signature as FLegalize, but would be applied at the later point. This would allow LowerRuntimeBuiltin to replace anything that has the FLowerBuiltin attribute, and wouldn't require a distinction between different levels of FLegalize.
I think having `FLowerBuiltin` builtin attribute is great. lets go with 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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Relax] Implement R.ensure_aligned and update memory planning for R.view [tvm]
Posted by "Lunderberg (via GitHub)" <gi...@apache.org>.
Lunderberg commented on code in PR #17145:
URL: https://github.com/apache/tvm/pull/17145#discussion_r1672412396
##########
python/tvm/relax/op/memory/view.py:
##########
@@ -92,3 +92,20 @@ def _normalize(expr, relax_cls):
relative_byte_offset = _normalize(relative_byte_offset, PrimValue)
return _ffi_api.view(data, shape, dtype, relative_byte_offset) # type: ignore
+
+
+def ensure_aligned(data: Expr) -> Expr:
Review Comment:
Can we rename this to `ensure_zero_offset`? The alignment for a view depends on whether `byte_offset % desired_alignment == 0`, not whether `byte_offset==0`. That is, a buffer with a non-zero `byte_offset` can be aligned.
##########
src/relax/op/memory/view.cc:
##########
@@ -355,5 +354,28 @@ TVM_REGISTER_OP("relax.memory.view")
.set_attr<FLegalize>("FLegalize", LegalizeView)
.set_attr<Bool>("FPurity", Bool(true));
+Expr ensure_aligned(const Expr& x) {
Review Comment:
Same name change request here, to replace `ensure_aligned` with `ensure_zero_offset`.
##########
src/runtime/relax_vm/builtin.cc:
##########
@@ -545,6 +545,19 @@ TVM_REGISTER_GLOBAL("vm.builtin.tensor_to_shape").set_body_typed([](NDArray data
return ShapeTuple(out_shape);
});
+TVM_REGISTER_GLOBAL("vm.builtin.ensure_aligned").set_body_typed([](NDArray data) {
+ if (data->byte_offset == 0) {
+ return data;
+ }
+ DLManagedTensor* dl_tensor = data.ToDLPack();
+ dl_tensor->dl_tensor.data =
+ reinterpret_cast<char*>(dl_tensor->dl_tensor.data) + dl_tensor->dl_tensor.byte_offset;
+ dl_tensor->dl_tensor.byte_offset = 0;
+ // For platforms that does not support pointer arithmetic, we need to copy the data to a new
+ // buffer.
+ return NDArray::FromDLPack(dl_tensor);
Review Comment:
This constructs a new `NDArray`, but does not actually copy the data. Instead, `NDArray::FromDLPack` takes ownership of the `DLManagedTensor*`. The pointer arithmetic performed earlier would still be invalid.
##########
src/relax/backend/vm/vm_builtin_lower.cc:
##########
@@ -124,6 +128,19 @@ class VMBuiltinLowerMutator : public ExprMutator {
}
}
+ Expr View(const Call& view_node) {
Review Comment:
The handling of `R.memory.view` is already handled by `relax.transform.LegalizeOps`, and doesn't require any additional handling here.
##########
src/relax/transform/static_plan_block_memory.cc:
##########
@@ -286,8 +286,13 @@ class TokenAllocator1D {
std::vector<StorageToken> full_pool_;
};
-/*! \brief Check if the input op is "relax.reshape". */
-bool IsReshape(const Expr& op) { return op.same_as(Op::Get("relax.reshape")); }
+/*! \brief Check if the input op is a memory op that return the same buffer as the input buffer. */
Review Comment:
Nitpick: Replace "a memory op that return the same buffer" with "a memory op that **may** return the same buffer", since it isn't guaranteed whether `ensure_zero_offset` will return a copy or an alias.
##########
src/runtime/relax_vm/builtin.cc:
##########
@@ -545,6 +545,19 @@ TVM_REGISTER_GLOBAL("vm.builtin.tensor_to_shape").set_body_typed([](NDArray data
return ShapeTuple(out_shape);
});
+TVM_REGISTER_GLOBAL("vm.builtin.ensure_aligned").set_body_typed([](NDArray data) {
+ if (data->byte_offset == 0) {
+ return data;
+ }
+ DLManagedTensor* dl_tensor = data.ToDLPack();
+ dl_tensor->dl_tensor.data =
Review Comment:
This pointer addition is not valid on all targets. Can we add a method to the `DeviceAPI` that returns a boolean, indicating whether the host may perform pointer arithmetic on a `DLTensor::data` pointer?
##########
src/relax/op/memory/view.cc:
##########
@@ -334,13 +334,12 @@ Expr LegalizeView(const BlockBuilder& bb, const Call& call) {
relative_byte_offset = relax::PrimValue::Int64(0);
}
- StructInfoDeriveFunc infer_sinfo_env_func;
- infer_sinfo_env_func = EnvFunc::Get("tvm.relax.struct_info.infer_view_sinfo");
- auto runtime_view_sinfo = FuncStructInfo::OpaqueFunc(infer_sinfo_env_func, true);
-
- ExternFunc runtime_view_func("runtime.TVMArrayCreateView", runtime_view_sinfo);
+ if (shape.same_as(call->args[1]) && dtype.same_as(call->args[2]) &&
+ relative_byte_offset.same_as(call->args[3])) {
+ return call;
+ }
- return Call(runtime_view_func, {data, shape, dtype, relative_byte_offset});
+ return Call(call->op, {data, shape, dtype, relative_byte_offset});
Review Comment:
This change means that `R.memory.view` is still present in the output of `LegalizeOps`, but a legalization function should replace the operator with a lowered form.
##########
tests/python/relax/test_op_view.py:
##########
@@ -483,18 +483,7 @@ def main(A: R.Tensor([4096], "float32")):
class Expected:
@R.function
def main(A: R.Tensor([4096], "float32")):
- B = R.ExternFunc(
- "runtime.TVMArrayCreateView",
- R.Callable(
- derive_func="tvm.relax.struct_info.infer_view_sinfo",
- purity=True,
- ),
- )(
- A,
- R.shape([64, 64]),
- R.dtype("float32"),
- R.prim_value(0),
- )
+ B = R.memory.view(A, shape=R.shape([64, 64]), dtype="float32", relative_byte_offset=0)
Review Comment:
I don't think this is the desired behavior for `R.memory.view`, as the operator hasn't actually been legalized.
--
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
Re: [PR] [Relax] Implement R.ensure_aligned and update memory planning for R.view [tvm]
Posted by "Lunderberg (via GitHub)" <gi...@apache.org>.
Lunderberg commented on PR #17145:
URL: https://github.com/apache/tvm/pull/17145#issuecomment-2220768515
Also, if you're interested, I have a partial implementation in [this dev branch](https://github.com/apache/tvm/compare/main...Lunderberg:temp_relax_implement_ensure_aligned_operator) that includes the device-type validation and a TIR legalization. If you'd like to pull any of it over, you're welcome to it, as I've had it on the back-burner for far too long.
--
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
Re: [PR] [Relax] Implement R.ensure_aligned and update memory planning for R.view [tvm]
Posted by "tqchen (via GitHub)" <gi...@apache.org>.
tqchen commented on PR #17145:
URL: https://github.com/apache/tvm/pull/17145#issuecomment-2220972695
Based on the current grouping, seems quite a bit of the runtime function dispatchings happens in LowerBuiltin, while legalizeOps primarily focused on lowering to TIR related functions.
I think such distinction is still helpful, so that can be a factor considering moving the view legalization into the VMBuiltin.
--
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
Re: [PR] [Relax] Implement R.ensure_aligned and update memory planning for R.view [tvm]
Posted by "tqchen (via GitHub)" <gi...@apache.org>.
tqchen commented on PR #17145:
URL: https://github.com/apache/tvm/pull/17145#issuecomment-2221657695
There are different ways to look at this particular case.
For this particular case, given the view was lowered to runtime function, it was primarily focused for the VM itself. One can also envision in future we have a codegen approach to get a view function that get inlined which is not needed in the VM approach.
Introducing legalize ops with different levels can also have extra issues, as we need to run default scheduling of some of the ops. But for the certain legalization level we do not have to.
Perhaps one way to make it more clear is to rename LowerVMBuiltin to LowerRuntimeBuiltin, which have clear indication that that is a pass which takes charge of lowering all implementaitons of runtime builtin functions.
--
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