Skip to content

Conversation

@Lunderberg
Copy link
Contributor

Prior to this commit, the Relax well-formed checker validated arguments provided to Relax functions, but did not validate arguments provided to R.call_tir. As a result, incorrect arguments from Relax to TIR would not be checked until runtime, if at all.

This commit updates the well-formed checker to verify that R.call_tir has received the correct arguments, and has the correct output shape specified in the out_sinfo parameter.

Prior to this commit, the Relax well-formed checker validated
arguments provided to Relax functions, but did not validate arguments
provided to `R.call_tir`.  As a result, incorrect arguments from Relax
to TIR would not be checked until runtime, if at all.

This commit updates the well-formed checker to verify that
`R.call_tir` has received the correct arguments, and has the correct
output shape specified in the `out_sinfo` parameter.
@Lunderberg
Copy link
Contributor Author

The implementation in this PR is similar to #17216, but with a few key changes:

@Lunderberg Lunderberg requested a review from tqchen August 19, 2024 21:04
@Lunderberg
Copy link
Contributor Author

All CI tests are passing, with the exception of the CI / Windows step, which has the same failures as seen across all PRs (currently being debugged in #17283). Since the CI / Windows is not a required step, this PR is ready for review/merge.

Initial implementation performed the validation as part of
`FNormalize`, to maximize coverage of this check.  This increased
end-to-end compilation time by ~10%, and so the check was requested to
be restricted to the well-formed checker.  Expensive operator-specific
validation is now performed in the new `FValidate` attribute.
@Lunderberg
Copy link
Contributor Author

@tvm-bot rerun

@Lunderberg
Copy link
Contributor Author

Looks like the failure on hexagon/pr-head is due to a timeout, not an error introduced in this PR. Of the 8 shards that run Hexagon unit tests, 7 of them finish in about 1h20m. The 8th node looks like it's being killed after 2 hours, and spends about 50 minutes running tests/python/contrib/test_hexagon/test_run_unit_tests.py::test_run_unit_tests[].

#17334 should resolve the hexagon CI issues by breaking up the single gigantic test, allowing them to be sharded out.

@Lunderberg
Copy link
Contributor Author

@tvm-bot rerun

Since #17334 has landed, the hexagon tests should be better spread out across the 8 shards.

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.

2 participants