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

[Codegen][Tuner] Allow tuning specs in the LLVMGPU pipeline #19359

Merged
merged 2 commits into from
Dec 4, 2024

Conversation

kuhar
Copy link
Member

@kuhar kuhar commented Dec 3, 2024

This adds the materialize-tuning-specs pass to the LLVMGPU executable configuration pipelines.

Add a test that shows that the tuning spec gets applied and picked up in the ROCDL pipeline.

Also, replace the print-based checks in existing tests with op remarks on transform strategy application in materialize-user-configs.

Issue: #19214

This adds the `materialize-tuning-specs` pass to the LLVMGPU executable
configuration pipelines.

Add a test that shows that the tuning spec gets applied and picked up in
the ROCDL pipeline.

Also, replace the print-based checks in existing tests with op remarks
on transform strategy application in `materialize-user-configs`.

Signed-off-by: Jakub Kuderski <[email protected]>
@kuhar kuhar force-pushed the tuning-spec-pipeline branch from 3b95e16 to b98f645 Compare December 3, 2024 20:57
Signed-off-by: Jakub Kuderski <[email protected]>
Copy link
Contributor

@Groverkss Groverkss left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines -28 to +34
// PARENT-LABEL: [ IR printer: Hello Tuning Spec top-level ]
// PARENT-NEXT: func.func @main_0
//
// PARENT-LABEL: module @parent attributes {
// PARENT-SAME: iree_codegen.tuning_spec_mlirbc = dense<
// PARENT-LABEL: module @child {
// PARENT: func.func @main_0

module @parent {
module @child {
// expected-remark@+1 {{Applied transform configuration strategy @iree_linked_tuning_spec::@__kernel_config}}
Copy link
Contributor

Choose a reason for hiding this comment

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

this is much nicer

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks for the suggestion

@kuhar kuhar merged commit 9f8aad8 into iree-org:main Dec 4, 2024
38 checks passed
@@ -1183,6 +1183,7 @@ static void buildLLVMGPUCodegenConfigurationPassPipelineImpl(
funcPassManager.addPass(createConfigTrackingCanonicalizerPass);
funcPassManager.addPass(createCSEPass);
}
modulePassManager.addPass(createMaterializeTuningSpecsPass());
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the pass ordering might be off... Do we want to do user configs first and then tuning spec. User config should be the ultimate override.

Copy link
Member Author

@kuhar kuhar Dec 4, 2024

Choose a reason for hiding this comment

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

User configs do come first. It's documented in the materialize user configs pass. Materialize tuning specs doesn't run the transform dialect interpreter and it's up to the materialize user configs to decide which transforms library to run.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe I need to come up with a different name to better disambiguate these two passes...

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