Skip to content

Conversation

@Lunderberg
Copy link
Contributor

Prior to this commit, an operator's FCallPacked attribute, used to specify a 1:1 mapping between a relax operator and a PackedFunc that implements it, was only checked in CodegenVM. Any operator with FCallPacked would raise an error when compiled using CodegenVMTIR.

This commit removes the FCallPacked handling from CodegenVM altogether, and instead checks for this attribute as part of LegalizeOps. This provides the same functionality across both backends.

@Lunderberg
Copy link
Contributor Author

@tvm-bot rerun

@Lunderberg Lunderberg requested a review from yongwww June 18, 2024 18:59
Copy link

@MadFunMaker MadFunMaker left a comment

Choose a reason for hiding this comment

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

LGTM

@tqchen
Copy link
Member

tqchen commented Sep 5, 2024

@Lunderberg need rebase

Prior to this commit, an operator's `FCallPacked` attribute, used to
specify a 1:1 mapping between a relax operator and a `PackedFunc` that
implements it, was only checked in `CodegenVM`.  Any operator with
`FCallPacked` would raise an error when compiled using `CodegenVMTIR`.

This commit removes the `FCallPacked` handling from `CodegenVM`
altogether, and instead checks for this attribute as part of
`LegalizeOps`.  This provides the same functionality across both
backends.
@Lunderberg Lunderberg force-pushed the bugfix_fcallpacked_in_codegen_vm_tir branch from fdfd30a to 25d1944 Compare September 5, 2024 16:44
@Lunderberg
Copy link
Contributor Author

Lunderberg commented Sep 5, 2024

Thank you, and rebased to resolve conflicts. (And again to resolve a conflict that wasn't flagged by git.)

@Lunderberg Lunderberg merged commit 4eafd00 into apache:main Sep 6, 2024
@Lunderberg Lunderberg deleted the bugfix_fcallpacked_in_codegen_vm_tir branch September 6, 2024 14:46
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