Skip to content

Conversation

@CharlieFRuan
Copy link
Member

In a recent PR #16183, TVMArgTypeCode introduced TVMArgBool. This PR is needed for the web runtime when a TVM packed function returns a bool.

case ArgTypeCode.Int:
case ArgTypeCode.UInt:
case ArgTypeCode.TVMArgBool:
return this.memory.loadI64(rvaluePtr);
Copy link
Member Author

Choose a reason for hiding this comment

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

cc @tqchen, would loadI64() be the right function to use for bool?

Copy link
Member

Choose a reason for hiding this comment

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

this would need to wait until #17240, but once the new bool type lands yes this would be the case

@tqchen
Copy link
Member

tqchen commented Aug 7, 2024

temp block this in light of #16183 (comment)

@Lunderberg
Copy link
Contributor

#17240 has now landed, so this PR should be able to be revisited.

@tqchen tqchen merged commit 1518008 into apache:main Aug 23, 2024
CharlieFRuan added a commit to mlc-ai/web-llm that referenced this pull request Aug 23, 2024
### Change
- #555

### TVMjs
- Updated to current head:
apache/tvm@1518008
  - Main change is apache/tvm#17251
- This is needed for WASMs compiled after
apache/tvm#17257 is merged (e.g. Phi-3.5). TVM
global functions that returns bool need this PR to run correctly (e.g.
`AcceptToken()` in BNFGrammar) in runtime.
- However, these are backward compatible to WASMs compiled prior to this
PR. Tested with Phi-3 (old WASM) running grammar.
jingyi-zhao-01 pushed a commit to jingyi-zhao-01/web-llm that referenced this pull request Dec 8, 2024
### Change
- mlc-ai#555

### TVMjs
- Updated to current head:
apache/tvm@1518008
  - Main change is apache/tvm#17251
- This is needed for WASMs compiled after
apache/tvm#17257 is merged (e.g. Phi-3.5). TVM
global functions that returns bool need this PR to run correctly (e.g.
`AcceptToken()` in BNFGrammar) in runtime.
- However, these are backward compatible to WASMs compiled prior to this
PR. Tested with Phi-3 (old WASM) running grammar.
atebites-hub pushed a commit to atebites-hub/web-llm that referenced this pull request Oct 4, 2025
### Change
- mlc-ai#555

### TVMjs
- Updated to current head:
apache/tvm@1518008
  - Main change is apache/tvm#17251
- This is needed for WASMs compiled after
apache/tvm#17257 is merged (e.g. Phi-3.5). TVM
global functions that returns bool need this PR to run correctly (e.g.
`AcceptToken()` in BNFGrammar) in runtime.
- However, these are backward compatible to WASMs compiled prior to this
PR. Tested with Phi-3 (old WASM) running grammar.
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