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

[BYOC] RelayToTIR custom codegen passes can still depend on dynamic shape functions #11619

Merged
merged 1 commit into from
Jun 10, 2022

Conversation

mbs-octoml
Copy link
Contributor

@mbs-octoml mbs-octoml commented Jun 7, 2022

In #11474 I got ready to switch CUTLASS from function-at-a-time to IRModule-at-a-time compilation.
However my approach didn't handle dynamic shape functions, so I adjust it here.

The idea is still that such passes will leave behind
calls to 'extern' functions. However, converting those
calls to 'call_lowered' form in
MarkCompilerFunctionsAsExtern is too soon since only
the TECompiler knows how to capture all the attributes
necessary to support dynamic shape functions.

So stop doing that in MarkCompilerFunctionsAsExtern and
instead support this case properly in the TECompiler.

While there try to chip away at the chronic lack of structure in te_compiler.cc. Every little bit helps.

Added a bare-bones unit test to exercise the main lowering/call-rewrite flavors, but much more is needed.

@mbs-octoml mbs-octoml marked this pull request as draft June 7, 2022 23:18
@mbs-octoml
Copy link
Contributor Author

(Will give this a run through ci in draft form).

@mbs-octoml mbs-octoml marked this pull request as ready for review June 8, 2022 00:56
Copy link
Contributor

@areusch areusch left a comment

Choose a reason for hiding this comment

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

thanks @mbs-octoml a couple questions for my understanding

@mbs-octoml mbs-octoml force-pushed the mbs-rewrite-extern-calls branch from 345b9db to 210e2c0 Compare June 8, 2022 18:58
@mbs-octoml
Copy link
Contributor Author

Thanks @areusch , I've added a large intro comment to capture what I've reverse engineered.

@mbs-octoml
Copy link
Contributor Author

mbs-octoml commented Jun 8, 2022

As discussed offline I'm going to pay down some testing tech debt and ping back.

@mbs-octoml mbs-octoml force-pushed the mbs-rewrite-extern-calls branch from 210e2c0 to db09ba6 Compare June 9, 2022 20:09
@mbs-octoml
Copy link
Contributor Author

@areusch PTAL, added a bare-bones-but-at-least-it's-a-start unit test in test_pass_lower_te.py.

Spent way too much time trying to make the tests of the form assert_equals(output, parse("...")) but gave up due to difficulties with matching GlobalVars.

…hape functions

In apache#11474 I got ready to switch CUTLASS from function-at-a-time to IRModule-at-a-time compilation.
However my approach didn't handle dynamic shape functions, so I adjust it here.

The idea is still that such passes will leave behind
calls to 'extern' functions. However, converting those
calls to 'call_lowered' form in
MarkCompilerFunctionsAsExtern is too soon since only
the TECompiler knows how to capture all the attributes
necessary to support dynamic shape functions.

So stop doing that in MarkCompilerFunctionsAsExtern and
instead support this case properly in the TECompiler.

While there try to chip away at the chronic lack of structure in te_compiler.cc. Every little bit helps.

Add a basic unit test.
@mbs-octoml mbs-octoml force-pushed the mbs-rewrite-extern-calls branch from feea55c to 78d92c8 Compare June 9, 2022 23:10
Copy link
Contributor

@areusch areusch left a comment

Choose a reason for hiding this comment

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

thanks @mbs-octoml !


main = actual_mod["main"]
call = main.body
assert call.op.name == "call_lowered"
Copy link
Contributor

Choose a reason for hiding this comment

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

i am sad :(

@areusch
Copy link
Contributor

areusch commented Jun 10, 2022

tagging issue describing test challenges to cross-reference it. #11661

@mbs-octoml
Copy link
Contributor Author

@tvm-bot merge

@github-actions
Copy link
Contributor

Cannot merge, these CI jobs are not successful on 78d92c8:

@masahi masahi merged commit ec24ae6 into apache:main Jun 10, 2022
Kathryn-cat pushed a commit to Kathryn-cat/tvm that referenced this pull request Jun 10, 2022
…hape functions (apache#11619)

In apache#11474 I got ready to switch CUTLASS from function-at-a-time to IRModule-at-a-time compilation.
However my approach didn't handle dynamic shape functions, so I adjust it here.

The idea is still that such passes will leave behind
calls to 'extern' functions. However, converting those
calls to 'call_lowered' form in
MarkCompilerFunctionsAsExtern is too soon since only
the TECompiler knows how to capture all the attributes
necessary to support dynamic shape functions.

So stop doing that in MarkCompilerFunctionsAsExtern and
instead support this case properly in the TECompiler.

While there try to chip away at the chronic lack of structure in te_compiler.cc. Every little bit helps.

Add a basic unit test.
@mbs-octoml mbs-octoml deleted the mbs-rewrite-extern-calls branch June 10, 2022 14:06
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