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

[FFI][Runtime] Use TVMValue::v_int64 to represent boolean values #17240

Merged

Conversation

Lunderberg
Copy link
Contributor

This is a follow-up to #16183, which added handling of boolean values in the TVM FFI. The initial implementation added both a new type code (kTVMArgBool) and a new TVMValue::v_bool variant. This commit removes the TVMValue::v_bool variant, since the kTVMArgBool type code is sufficient to handle boolean arguments.

Removing the TVMValue::v_bool variant also makes all TVMValue variants be 64-bit (assuming a 64-bit CPU). This can simplify debugging in some cases, since it prevents partial values from inactive variants from being present in memory.

This is a follow-up to apache#16183, which
added handling of boolean values in the TVM FFI.  The initial
implementation added both a new type code (`kTVMArgBool`) and a new
`TVMValue::v_bool` variant.  This commit removes the
`TVMValue::v_bool` variant, since the `kTVMArgBool` type code is
sufficient to handle boolean arguments.

Removing the `TVMValue::v_bool` variant also makes all `TVMValue`
variants be 64-bit (assuming a 64-bit CPU).  This can simplify
debugging in some cases, since it prevents partial values from
inactive variants from being present in memory.
@Lunderberg Lunderberg force-pushed the ffi_use_int64_variant_to_represent_bool branch from d2ac722 to 5ee4fb3 Compare August 19, 2024 13:48
@Lunderberg
Copy link
Contributor Author

Rebased onto main, to rerun CI with the extra fixes after the revert/reapply of the FFI changes in #17252 and #17257.

@Lunderberg
Copy link
Contributor Author

Fixed last two failing unit tests, since MakePackedAPI no longer needs to conditional load/convert a boolean to int64.

@tqchen tqchen merged commit 0f037a6 into apache:main Aug 22, 2024
18 of 19 checks passed
@Lunderberg Lunderberg deleted the ffi_use_int64_variant_to_represent_bool branch August 22, 2024 17:14
MasterJH5574 added a commit to MasterJH5574/tvm that referenced this pull request Aug 29, 2024
This PR fixes the conversion of boolean value for Java, following the
changes in apache#17240
MasterJH5574 added a commit to mlc-ai/relax that referenced this pull request Aug 29, 2024
This PR fixes the conversion of boolean value for Java, following the
changes in apache/tvm#17240
MasterJH5574 added a commit to MasterJH5574/tvm that referenced this pull request Aug 29, 2024
This PR fixes the conversion of boolean value for Java, following the
changes in apache#17240
MasterJH5574 added a commit to MasterJH5574/tvm that referenced this pull request Aug 29, 2024
This PR fixes the conversion of boolean value for Java, following the
changes in apache#17240
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