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

Compiled with Default Target(LLVM) and Built with USE_MRVL=ON #17455

Merged
merged 14 commits into from
Oct 25, 2024

Conversation

MNGanesan
Copy link
Contributor

Please review the fix:

It is a unique use-case to check the default target(LLVM), though TVM is built with BYOC(MRVL-ON)
The config of Codegen(BYOC) contains default values for configuration/optons, it is extracted
during _generate_codegen_args. In command line processing, validate_target_args checks if there are add-on options and it expects that particular target to be given explicitly in command line. Here, it is test for default (LLVM) path only, hence validate_target_args need to ignore the codegen's configuration

M N Ganesan added 8 commits October 9, 2024 13:09
… BYOC(apache#17454)

It is a unique use-case to check the default target(LLVM), though TVM is built with BYOC(MRVL-ON)
The config of Codegen(BYOC) contains default values for configuration/options, it is extracted
during _generate_codegen_args. In command line processing, validate_target_args checks if there
are add-on options and it expects that particular target to be given explicitly in command line.
Here, it is test for default (LLVM) path only, hence validate_target_args need to ignore the codegen's
configuration for default target.

Signed-off-by: M N Ganesan <muthusamynam@marvell.com>
… BYOC(apache#17454)

It is a unique use-case to check the default target(LLVM), though TVM is built with BYOC(MRVL-ON)
The config of Codegen(BYOC) contains default values for configuration/options, it is extracted
during _generate_codegen_args. In command line processing, validate_target_args checks if there
are add-on options and it expects that particular target to be given explicitly in command line.
Here, it is test for default (LLVM) path only, hence validate_target_args need to ignore the codegen's
configuration for default target.

Signed-off-by: M N Ganesan <muthusamynam@marvell.com>
… BYOC(apache#17454)

It is a unique use-case to check the default target(LLVM), though TVM is built with BYOC(MRVL-ON)
The config of Codegen(BYOC) contains default values for configuration/optons, it is extracted
during _generate_codegen_args. In command line processing, validate_target_args checks if there are add-on options and it expects that particular target to be given explicitly in command line.  Here, it is test for default (LLVM) path only, hence validate_target_args need to ignore the codegen's configuration
… BYOC(apache#17454)

It is a unique use-case to check the default target(LLVM), though TVM is built with BYOC(MRVL-ON)
The config of Codegen(BYOC) contains default values for configuration/optons, it is extracted
during _generate_codegen_args. In command line processing, validate_target_args checks if there are add-on options and it expects that particular target to be given explicitly in command line.  Here, it is test for default (LLVM) path only, hence validate_target_args need to ignore the codegen's configuration
…h BYOC(apache#17454)

It is a unique use-case to check the default target(LLVM), though TVM is built with BYOC(MRVL-ON)
The config of Codegen(BYOC) contains default values for configuration/optons, it is extracted
during _generate_codegen_args. In command line processing, validate_target_args checks if there are
add-on options and it expects that particular target to be given explicitly in command line.
Here, it is test for default (LLVM) path only, hence validate_target_args need to ignore the
codegen's configuration

Signed-off-by: M N Ganesan <muthusamynam@marvell.com>
…h BYOC(apache#17454)

    It is a unique use-case to check the default target(LLVM), though TVM is built with BYOC(MRVL-ON)
    The config of Codegen(BYOC) contains default values for configuration/optons, it is extracted
    during _generate_codegen_args. In command line processing, validate_target_args checks if there are
    add-on options and it expects that particular target to be given explicitly in command line.
    Here, it is test for default (LLVM) path only, hence validate_target_args need to ignore the
    codegen's configuration

Signed-off-by: M N Ganesan <muthusamynam@marvell.com>
…h BYOC(apache#17454)

        It is a unique use-case to check the default target(LLVM), though TVM is built with BYOC(MRVL-ON)
        The config of Codegen(BYOC) contains default values for configuration/optons, it is extracted
        during _generate_codegen_args. In command line processing, validate_target_args checks if there are
        add-on options and it expects that particular target to be given explicitly in command line.
        Here, it is test for default (LLVM) path only, hence validate_target_args need to ignore the
        codegen's configuration

Signed-off-by: M N Ganesan <muthusamynam@marvell.com>
@MNGanesan
Copy link
Contributor Author

@Hzfengsy @lhutton1

Could you please review this patch ?

Thanks

@Hzfengsy
Copy link
Member

cc TVMC maintainer @leandron

python/tvm/driver/tvmc/target.py Outdated Show resolved Hide resolved
M N Ganesan and others added 2 commits October 23, 2024 12:53
…_MRVL=ON(apache#17454)

This is a use-case of invoking TVMC with default target though it is built with MRVL_ON.
In command line processing, validate_target_args checks if there are add-on options
derived from the default arguments of codegen/BYOC and it expects that particular codegen
to be given explicitly in command line. However, certain codegen's can have default target alone,
in that case codegen optios are not extracted there by relaxing the validation

Signed-off-by: M N Ganesan <muthusamynam@marvell.com>
@MNGanesan
Copy link
Contributor Author

@leandron

Please review my updated patch which has generic fix.,
The purpose of this fix is not to extract any options of codegen(BYOC) when only default target is specified by the user.

Eg:
python3 -m tvm.driver.tvmc compile --target="llvm" model.onnx

Thanks

M N Ganesan added 4 commits October 24, 2024 09:28
…_MRVL=ON(apache#17454)

This is a use-case of invoking TVMC with default target though it is built with MRVL_ON.
In command line processing, validate_target_args checks if there are add-on options
derived from the default arguments of codegen/BYOC and it expects that particular codegen
to be given explicitly in command line. However, certain codegen's can have default target alone,
in that case codegen optios are not extracted there by relaxing the validation

Signed-off-by: M N Ganesan <muthusamynam@marvell.com>
…E_MRVL=ON(apache#17454)

    This is a use-case of invoking TVMC with default target though it is built with MRVL_ON.
    In command line processing, validate_target_args checks if there are add-on options
    derived from the default arguments of codegen/BYOC and it expects that particular codegen
    to be given explicitly in command line. However, certain codegen's can have default target alone,
    in that case codegen optios are not extracted there by relaxing the validation

Signed-off-by: M N Ganesan <muthusamynam@marvell.com>
Copy link
Contributor

@leandron leandron left a comment

Choose a reason for hiding this comment

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

OK, thanks, it looks very elegant and general now.

@MNGanesan
Copy link
Contributor Author

@leandron

Could you please merge this PR?

Thanks

@leandron leandron merged commit 988255e into apache:main Oct 25, 2024
20 of 21 checks passed
@leandron
Copy link
Contributor

@leandron

Could you please merge this PR?

Thanks

Yes, I just forgot this step after the review. This is merged now - thanks for your PR.

@MNGanesan
Copy link
Contributor Author

@leandron

Thanks a lot for your quick review feedback and response.

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