8000 Reduce unnecessary IRs from CodeGen encoding materialization tests · Issue #20825 · iree-org/iree · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Reduce unnecessary IRs from CodeGen encoding materialization tests #20825

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.

Sign up for GitHub

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

Jump to bottom
Open
hanhanW opened this issue May 15, 2025 · 3 comments
Open

Reduce unnecessary IRs from CodeGen encoding materialization tests #20825

hanhanW opened this issue May 15, 2025 · 3 comments
Assignees
@hanhanW
Labels
codegen Shared code generation infrastructure and dialects onboarding/codegen Tasks suitable for new team member onboarding quality of life 😊 Nice things are nice; let's have some

Comments

@hanhanW
Copy link
Contributor
hanhanW commented May 15, 2025

The existing tests and future lit tests add too many unnecessary IRs like tensor_ext ops and hal ops. The tricky part is that it uses type converter and it runs CSE within the pass. CSE is needed for more robust checks, as there could be many same constants created during the pass, etc.

I've been thinking how to reduce the burden for this, and my initial idea was adding optimization_barrier at the end of the function. Now I think maybe we should just add the support for func ops and return ops to complete the pass. The patterns are simple, and it saves a lot of IRs from the tests. IMO, we don't need to consider if they are valid inputs for codegen or not. It is more about completeness. In this case, the tests could be structured like in few categories:

  • Test actual conversion for linalg ops, with types/shapes variants. For types/shapes variants, this part can be unit tests, but I think compiler codebase like lit tests better.
  • Test the conversion for tensor_ext and binding ops.
  • Few tests for DAG inputs.

It also helps future support. E.g., people don't need to have those bindings in the new lit tests.

iree/compiler/src/iree/compiler/Codegen/Common/test/materialize_encoding_gfx950.mlir

Lines 11 to 27 in a96d9cf

// XXX I don't think these test names are correct but I don't understand what "unroll" has to do with anything XXX
#encoding = #iree_encoding.encoding<operand_index = 0, op_type = matmul, element_types = [i8, i8, i32],
user_indexing_maps = [affine_map<(d0, d1, d2) -> (d0, d2)>, affine_map<(d0, d1, d2) -> (d2, d1)>, affine_map<(d0, d1, d2) -> (d0, d1)>],
iteration_sizes = [255, 513, ?]>
#pipeline_layout = #hal.pipeline.layout<bindings = [
#hal.pipeline.binding<storage_buffer>,
#hal.pipeline.binding<storage_buffer>
]>
func.func @set_encoding_LHS_unroll8x8x2_MFMA_I32_16x16x64_I8() {
%c0 = arith.constant 0 : index
%0 = hal.interface.binding.subspan layout(#pipeline_layout) binding(0) alignment(64) offset(%c0) flags(ReadOnly) : !iree_tensor_ext.dispatch.tensor<readonly:tensor<255x513xi8>>
%1 = hal.interface.binding.subspan layout(#pipeline_layout) binding(1) alignment(64) offset(%c0) : !iree_tensor_ext.dispatch.tensor<writeonly:tensor<255x513xi8, #encoding>>
%2 = iree_tensor_ext.dispatch.tensor.load %0, offsets = [0, 0], sizes = [255, 513], strides = [1, 1] : !iree_tensor_ext.dispatch.tensor<readonly:tensor<255x513xi8>> -> tensor<255x513xi8>
%3 = iree_encoding.set_encoding %2 : tensor<255x513xi8> -> tensor<255x513xi8, #encoding>
iree_tensor_ext.dispatch.tensor.store %3, %1, offsets = [0, 0], sizes = [255, 513], strides = [1, 1] : tensor<255x513xi8, #encoding> -> !iree_tensor_ext.dispatch.tensor<writeonly:tensor<255x513xi8, #encoding>>
return
}

It can just be

// The iteration_sizes can be dropped in GPU tests thought, it is a different topic.
 #encoding = #iree_encoding.encoding<operand_index = 0, op_type = matmul, element_types = [i8, i8, i32], 
                                     user_indexing_maps = [affine_map<(d0, d1, d2) -> (d0, d2)>, affine_map<(d0, d1, d2) -> (d2, d1)>, affine_map<(d0, d1, d2) -> (d0, d1)>], 
                                     iteration_sizes = [255, 513, ?]> 
func.func @set_encoding_LHS_unroll8x8x2_MFMA_I32_16x16x64_I8(%2: tensor<255x513xi8>) -> tensor<255x513xi8, #encoding> {
  %3 = iree_encoding.set_encoding %2 : tensor<255x513xi8> -> tensor<255x513xi8, #encoding>
  return %3 : tensor<255x513xi8, #encoding>
}
@hanhanW hanhanW self-assigned this May 15, 2025
@hanhanW hanhanW added codegen Shared code generation infrastructure and dialects quality of life 😊 Nice things are nice; let's have some labels May 15, 2025
@benvanik
Copy link
Collaborator

+1! would be a nice guideline to have tests only include hal/tensor_ext ops if the pass was reliant on them - if not, they don't belong.

@hanhanW
Copy link
Contributor Author
hanhanW commented May 15, 2025

+1! would be a nice guideline to have tests only include hal/tensor_ext ops if the pass was reliant on them - if not, they don't belong.

I think they should only exist for few kind of tests. E.g., pipeline tests, bufferization, etc. This is something I want to take a look, but I don't have enough cycles on this. :)

@hanhanW hanhanW added the onboarding/codegen Tasks suitable for new team member onboarding label May 15, 2025
@hanhanW
Copy link
Contributor Author
hanhanW commented May 15, 2025

(Add the onboarding/codegen tag because adding the patterns can be a good starter tasks.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@hanhanW hanhanW

Labels
codegen Shared code generation infrastructure and dialects onboarding/codegen Tasks suitable for new team member onboarding quality of life 😊 Nice things are nice; let's have some
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@benvanik @hanhanW
0