Skip to content

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 <[email protected]>
… 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 <[email protected]>
… 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 <[email protected]>
…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 <[email protected]>
…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 <[email protected]>
@MNGanesan
Copy link
Contributor Author

@Hzfengsy @lhutton1

Could you please review this patch ?

Thanks

@Hzfengsy
Copy link
Member

cc TVMC maintainer @leandron

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 <[email protected]>
@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 <[email protected]>
…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 <[email protected]>
@MNGanesan MNGanesan requested a review from leandron October 24, 2024 10:20
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
@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