Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Relax] Require correct input/output shapes R.call_tir #17285

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.

@Lunderberg Lunderberg merged commit 491a0f6 into apache:main Sep 6, 2024
17 of 18 checks passed
@Lunderberg Lunderberg deleted the relax_check_call_tir_arguments_in_well_formed_checker branch September 6, 2024 14:49
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