Skip to content

Conversation

@srkreddy1238
Copy link
Contributor

Not all plarforms 64bit aligned allocations. Platforms with 32bit alignment fail to support set_input_zero_copy even though the ndarray is allocated by the tvm runtime itself.

This change enabled configurable option for such targets.

@tvm-bot
Copy link
Collaborator

tvm-bot commented Nov 7, 2022

Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.

Generated by tvm-bot

Not all plarforms 64bit aligned allocations. Platforms with 32bit alignment fail to support
set_input_zero_copy even though the ndarray is allocated by the tvm runtime itself.

This change enabled configurable option for such targets.
Copy link
Member

@junrushao junrushao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@srkreddy1238
Copy link
Contributor Author

cc @junrushao

Copy link
Member

@junrushao junrushao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@junrushao junrushao merged commit 24790d1 into apache:main Nov 15, 2022
@junrushao
Copy link
Member

Thanks @srkreddy1238 for the PR!

xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
…3307)

Not all plarforms 64bit aligned allocations. Platforms with 32bit alignment fail to support
set_input_zero_copy even though the ndarray is allocated by the tvm runtime itself.

This change enabled configurable option for such targets.

Co-authored-by: Siva Rama Krishna Reddy B <sivb@blr-ubuntu-ripper.qualcomm.com>
Lunderberg added a commit to Lunderberg/tvm that referenced this pull request May 4, 2023
Prior to this commit, any use of `tvm.nd.from_dlpack` to create a
strided `NDArray`, or a `NDArray` whose alignment was less than
`tvm::runtime::kAllocAlignment` would raise an error.  As a result,
views into larger arrays, which are unlikely to be aligned and
compact, could only be shared when copied into an aligned and compact
buffer.

This commit moves the compact/aligned check from the `NDArray` class
into the generated TIR code as part of DLTensor unpacking.  These
checks were initially introduced in
apache#11391, to avoid segfaults caused by
use of non-aligned buffers in code intended for aligned buffers.  The
new checks will provide the same safeguard as the alignment is checked
prior to use, but allows the alignment requirement to be relaxed on a
per-buffer basis.

This approach also removes a potential bug resulting from compile-time
configuration of `tvm::runtime::kAllocAlignment`, first introduced in
apache#13307.  Since TVM supports
cross-compiling, the installation of TVM used to compile a kernel may
assume a larger value of `kAllocAlignment` than is provided by the
runtime installation of TVM.  By validating the alignment within the
generated kernel, rather than as part of the runtime, this potential
inconsistency would be caught.
Lunderberg added a commit to Lunderberg/tvm that referenced this pull request May 8, 2023
Prior to this commit, any use of `tvm.nd.from_dlpack` to create a
strided `NDArray`, or a `NDArray` whose alignment was less than
`tvm::runtime::kAllocAlignment` would raise an error.  As a result,
views into larger arrays, which are unlikely to be aligned and
compact, could only be shared when copied into an aligned and compact
buffer.

This commit moves the compact/aligned check from the `NDArray` class
into the generated TIR code as part of DLTensor unpacking.  These
checks were initially introduced in
apache#11391, to avoid segfaults caused by
use of non-aligned buffers in code intended for aligned buffers.  The
new checks will provide the same safeguard as the alignment is checked
prior to use, but allows the alignment requirement to be relaxed on a
per-buffer basis.

This approach also removes a potential bug resulting from compile-time
configuration of `tvm::runtime::kAllocAlignment`, first introduced in
apache#13307.  Since TVM supports
cross-compiling, the installation of TVM used to compile a kernel may
assume a larger value of `kAllocAlignment` than is provided by the
runtime installation of TVM.  By validating the alignment within the
generated kernel, rather than as part of the runtime, this potential
inconsistency would be caught.
Lunderberg added a commit to Lunderberg/tvm that referenced this pull request Jun 5, 2023
Prior to this commit, any use of `tvm.nd.from_dlpack` to create a
strided `NDArray`, or a `NDArray` whose alignment was less than
`tvm::runtime::kAllocAlignment` would raise an error.  As a result,
views into larger arrays, which are unlikely to be aligned and
compact, could only be shared when copied into an aligned and compact
buffer.

This commit moves the compact/aligned check from the `NDArray` class
into the generated TIR code as part of DLTensor unpacking.  These
checks were initially introduced in
apache#11391, to avoid segfaults caused by
use of non-aligned buffers in code intended for aligned buffers.  The
new checks will provide the same safeguard as the alignment is checked
prior to use, but allows the alignment requirement to be relaxed on a
per-buffer basis.

This approach also removes a potential bug resulting from compile-time
configuration of `tvm::runtime::kAllocAlignment`, first introduced in
apache#13307.  Since TVM supports
cross-compiling, the installation of TVM used to compile a kernel may
assume a larger value of `kAllocAlignment` than is provided by the
runtime installation of TVM.  By validating the alignment within the
generated kernel, rather than as part of the runtime, this potential
inconsistency would be caught.
Lunderberg added a commit to Lunderberg/tvm that referenced this pull request Jun 12, 2023
Prior to this commit, any use of `tvm.nd.from_dlpack` to create a
strided `NDArray`, or a `NDArray` whose alignment was less than
`tvm::runtime::kAllocAlignment` would raise an error.  As a result,
views into larger arrays, which are unlikely to be aligned and
compact, could only be shared when copied into an aligned and compact
buffer.

This commit moves the compact/aligned check from the `NDArray` class
into the generated TIR code as part of DLTensor unpacking.  These
checks were initially introduced in
apache#11391, to avoid segfaults caused by
use of non-aligned buffers in code intended for aligned buffers.  The
new checks will provide the same safeguard as the alignment is checked
prior to use, but allows the alignment requirement to be relaxed on a
per-buffer basis.

This approach also removes a potential bug resulting from compile-time
configuration of `tvm::runtime::kAllocAlignment`, first introduced in
apache#13307.  Since TVM supports
cross-compiling, the installation of TVM used to compile a kernel may
assume a larger value of `kAllocAlignment` than is provided by the
runtime installation of TVM.  By validating the alignment within the
generated kernel, rather than as part of the runtime, this potential
inconsistency would be caught.
Lunderberg added a commit to Lunderberg/tvm that referenced this pull request Jul 5, 2023
Prior to this commit, any use of `tvm.nd.from_dlpack` to create a
strided `NDArray`, or a `NDArray` whose alignment was less than
`tvm::runtime::kAllocAlignment` would raise an error.  As a result,
views into larger arrays, which are unlikely to be aligned and
compact, could only be shared when copied into an aligned and compact
buffer.

This commit moves the compact/aligned check from the `NDArray` class
into the generated TIR code as part of DLTensor unpacking.  These
checks were initially introduced in
apache#11391, to avoid segfaults caused by
use of non-aligned buffers in code intended for aligned buffers.  The
new checks will provide the same safeguard as the alignment is checked
prior to use, but allows the alignment requirement to be relaxed on a
per-buffer basis.

This approach also removes a potential bug resulting from compile-time
configuration of `tvm::runtime::kAllocAlignment`, first introduced in
apache#13307.  Since TVM supports
cross-compiling, the installation of TVM used to compile a kernel may
assume a larger value of `kAllocAlignment` than is provided by the
runtime installation of TVM.  By validating the alignment within the
generated kernel, rather than as part of the runtime, this potential
inconsistency would be caught.

This check is also restricted to targets whose `void*` opaque pointer
can be interpreted as a pointer to the data array.  (e.g. No such
check applies on Vulkan, as the `void*` is a pointer to a struct that
contains additional bookkeeping.)
Lunderberg added a commit to Lunderberg/tvm that referenced this pull request May 23, 2024
Prior to this commit, any use of `tvm.nd.from_dlpack` to create a
strided `NDArray`, or a `NDArray` whose alignment was less than
`tvm::runtime::kAllocAlignment` would raise an error.  As a result,
views into larger arrays, which are unlikely to be aligned and
compact, could only be shared when copied into an aligned and compact
buffer.

This commit moves the compact/aligned check from the `NDArray` class
into the generated TIR code as part of DLTensor unpacking.  These
checks were initially introduced in
apache#11391, to avoid segfaults caused by
use of non-aligned buffers in code intended for aligned buffers.  The
new checks will provide the same safeguard as the alignment is checked
prior to use, but allows the alignment requirement to be relaxed on a
per-buffer basis.

This approach also removes a potential bug resulting from compile-time
configuration of `tvm::runtime::kAllocAlignment`, first introduced in
apache#13307.  Since TVM supports
cross-compiling, the installation of TVM used to compile a kernel may
assume a larger value of `kAllocAlignment` than is provided by the
runtime installation of TVM.  By validating the alignment within the
generated kernel, rather than as part of the runtime, this potential
inconsistency would be caught.

This check is also restricted to targets whose `void*` opaque pointer
can be interpreted as a pointer to the data array.  (e.g. No such
check applies on Vulkan, as the `void*` is a pointer to a struct that
contains additional bookkeeping.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants