Re: [PR] [TIR] Fix segfaults from ordering of Let/Assert in MakePackedAPI [tvm]
Lunderberg merged PR #16543: URL: https://github.com/apache/tvm/pull/16543 -- 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-unsubscr...@tvm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [TIR] Fix segfaults from ordering of Let/Assert in MakePackedAPI [tvm]
Lunderberg commented on PR #16543: URL: https://github.com/apache/tvm/pull/16543#issuecomment-2030270161 Last CI failure is due to a flaky hexagon test. I've added a `@pytest.mark.skip` to this PR, now just waiting on CI. -- 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-unsubscr...@tvm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [TIR] Fix segfaults from ordering of Let/Assert in MakePackedAPI [tvm]
Lunderberg commented on code in PR #16543:
URL: https://github.com/apache/tvm/pull/16543#discussion_r1539344129
##
src/tir/transforms/arg_binder.cc:
##
@@ -186,18 +191,8 @@ void ArgBinder::BindDLTensor(const Buffer& buffer, const
PrimExpr& device_type,
if (!(buffer->dtype == DataType::Int(1) || buffer->dtype == DataType::Int(4)
||
buffer->dtype == DataType::UInt(4))) {
auto type_msg = tvm::tir::StringImm(type_err_msg.str());
-asserts_.emplace_back(AssertStmt(a_ndim == v_ndim, msg, nop));
Review Comment:
Yup. The buffer's dimensionality is checked earlier, so this is entirely a
duplicate check on the dimensionality.
--
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-unsubscr...@tvm.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [TIR] Fix segfaults from ordering of Let/Assert in MakePackedAPI [tvm]
Lunderberg commented on code in PR #16543:
URL: https://github.com/apache/tvm/pull/16543#discussion_r1539338859
##
rust/tvm-graph-rt/tests/test_tvm_basic/build.rs:
##
@@ -48,10 +48,6 @@ fn main() -> Result<()> {
obj_file.exists(),
"Could not build tvm lib: {}",
String::from_utf8(output.stderr)?
-.trim()
-.split("\n")
-.last()
-.unwrap_or("")
Review Comment:
Oh, that's really weird. I'm guessing it was from bouncing over to the PR
branch of https://github.com/apache/tvm/pull/16183, which touched a number of
the FFI bindings. I've removed this delta from the PR.
--
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-unsubscr...@tvm.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [TIR] Fix segfaults from ordering of Let/Assert in MakePackedAPI [tvm]
slyubomirsky commented on code in PR #16543:
URL: https://github.com/apache/tvm/pull/16543#discussion_r1538390306
##
src/tir/transforms/arg_binder.cc:
##
@@ -186,18 +191,8 @@ void ArgBinder::BindDLTensor(const Buffer& buffer, const
PrimExpr& device_type,
if (!(buffer->dtype == DataType::Int(1) || buffer->dtype == DataType::Int(4)
||
buffer->dtype == DataType::UInt(4))) {
auto type_msg = tvm::tir::StringImm(type_err_msg.str());
-asserts_.emplace_back(AssertStmt(a_ndim == v_ndim, msg, nop));
Review Comment:
Was this just a duplicate?
##
rust/tvm-graph-rt/tests/test_tvm_basic/build.rs:
##
@@ -48,10 +48,6 @@ fn main() -> Result<()> {
obj_file.exists(),
"Could not build tvm lib: {}",
String::from_utf8(output.stderr)?
-.trim()
-.split("\n")
-.last()
-.unwrap_or("")
Review Comment:
Wonder how this came up. Just for readability?
--
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-unsubscr...@tvm.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [TIR] Fix segfaults from ordering of Let/Assert in MakePackedAPI [tvm]
Lunderberg commented on PR #16543: URL: https://github.com/apache/tvm/pull/16543#issuecomment-1936580514 Merged from main to PR branch, to resolve CI breakage that was fixed in https://github.com/apache/tvm/pull/16546. -- 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-unsubscr...@tvm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [TIR] Fix segfaults from ordering of Let/Assert in MakePackedAPI [tvm]
Lunderberg commented on PR #16543: URL: https://github.com/apache/tvm/pull/16543#issuecomment-1933217017 This came about while debugging the implementation of https://github.com/apache/tvm/pull/16542, but is otherwise unrelated. -- 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-unsubscr...@tvm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] [TIR] Fix segfaults from ordering of Let/Assert in MakePackedAPI [tvm]
Lunderberg opened a new pull request, #16543: URL: https://github.com/apache/tvm/pull/16543 Prior to this commit, the `MakePackedAPI` pass would output steps in the following order: 1. Check the number of arguments. 2. All `LetStmt` produced by the `ArgBinder` 3. `AssertStmt` for the Type code checks for each argument. 4. Additional `AssertStmt` produced by the `ArgBinder`. This order can cause segfaults if a function was provided incorrect arguments. For example, an integer argument passed to a function expecting a `DLTensor*` would be dereferenced to find the tensor's data pointer (step (2)) before checking if it is valid to perform that dereference (step (3)). The same would occur when reading the size of a tensor's axes (step (2)) before checking whether the tensor is the correct dimensionality (step (4)). This commit updates the steps to the following order. 1. Check the number of arguments. 2. Check the type code of each argument. 3. All `LetStmt` and `AssertStmt` produced by the `ArgBinder`, in the order in which they are generated. -- 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-unsubscr...@tvm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
