8000 Switching to get encoding resolver from configuration · Issue #20202 · iree-org/iree · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Switching to get encoding resolver from configuration #20202

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
Closed
hanhanW opened this issue Mar 10, 2025 · 4 comments · Fixed by #20271
Closed

Switching to get encoding resolver from configuration #20202

hanhanW opened this issue Mar 10, 2025 · 4 comments · Fixed by #20271
Assignees
@hanhanW @Max191

Comments

@hanhanW
Copy link
Contributor
hanhanW commented Mar 10, 2025

The MaterializeEncoding pass currently uses the encoding resolver based on the target attribute. However, we should get the resolver from the target configuration, if any. This is not only needed for multi-device, but also for AMD GPU data-tiling work. Because currently there are two encoding resolvers. One is GPUPadEncoding attr resolver and the other is the data-tiling encoding resolver. There is a confusion about which resolver should be used on GPU path. Once we finish #20160 and switch it to get it from configuration. There are no confusions.

iree/compiler/src/iree/compiler/Codegen/Common/MaterializeEncoding.cpp

Lines 70 to 98 in 74458b3

if (isVMVXBackend(targetAttr)) {
LDBG("Select VMVXEncodingLayoutAttr attribute as the layout attribute.");
layoutAttr = cast<IREE::Codegen::LayoutAttrInterface>(
IREE::CPU::VMVXEncodingLayoutAttr::get(
ctx, targetAttr.getConfiguration()));
} else if (isLLVMCPUBackend(targetAttr)) {
LDBG("Select CPUEncodingLayoutAttr attribute as the layout attribute.");
layoutAttr = cast<IREE::Codegen::LayoutAttrInterface>(
IREE::CPU::CPUEncodingLayoutAttr::get(ctx,
targetAttr.getConfiguration()));
} else if (isROCMBackend(targetAttr)) {
LDBG("Select GPUEncodingLayoutAttr attribute as the layout attribute.");
layoutAttr = cast<IREE::Codegen::LayoutAttrInterface>(
IREE::GPU::GPUEncodingLayoutAttr::get(
ctx, DictionaryAttr::get(
ctx, NamedAttribute(kGPUTargetAttrName,
getGPUTargetAttr(targetAttr)))));
} else if (testCLGPUTarget) {
LDBG("Select GPUEncodingLayoutAttr attribute as the layout attribute. "
"(testCLGPUTarget)");
layoutAttr = cast<IREE::Codegen::LayoutAttrInterface>(
IREE::GPU::GPUEncodingLayoutAttr::get(
ctx,
DictionaryAttr::get(ctx, NamedAttribute(kGPUTargetAttrName,
getCLGPUTarget(ctx)))));
} else {
LDBG("Select EncodingNopLayoutAttr attribute as the layout attribute.");
layoutAttr = IREE::Codegen::EncodingNopLayoutAttr::get(ctx);
}

Note: it does not depend on #20160. They can happen together, and the trade-off is having two cmd flags to enable the data-tiling approach.
@hanhanW
Copy link
Contributor Author
hanhanW commented Mar 10, 2025

@jtuyls we talked about getting the encoding resolver from the target configuration. Sorry, I don't remember if you're looking at it or not. CC you just in case if you're looking at it.

@jtuyls
Copy link
Contributor
jtuyls commented Mar 10, 2025

@jtuyls we talked about getting the encoding resolver from the target configuration. Sorry, I don't remember if you're looking at it or not. CC you just in case if you're looking at it.

No, I wasn't looking at this, but thanks for the heads up! I will not start looking at this then.

@Max191 Max191 self-assigned this Mar 14, 2025
@Max191
Copy link
Contributor
Max191 commented Mar 14, 2025

I'll pick this issue up

@Max191
Copy link
Contributor
Max191 commented Mar 14, 2025

Related PR: #20250

This was referenced Mar 14, 2025
Merged
Merged
Max191 added a commit that referenced this issue Mar 17, 2025
@Max191
[DataTiling][GPU] Add layout resolver option to ROCMTarget (#20250)
73ee6d5 
Add a new flag option to ROCMTarget called
`iree-hip-encoding-layout-resolver`. The option selects between the
different layout resolvers for encoding attributes:
 - `none`: Select the default layout resolver (materialize to nop)
- `pad`: Select the GPUPadLayoutAttr resolver (materialize into padding)
- `data-tiling`: Select the GPUEncodingLayoutAttr resolver (data tiling
materialization)

The `iree-hip-enable-experimental-pad-layout` and
`iree-llvmgpu-experimental-data-tiling` are removed because they are
folded into the new option.

Currently, we run both the `MaterializeDeviceEncodingPass` and the
`MaterializeEncodingIntoPaddingPass` because the padding layout
materialization cannot be handled by the MaterializeDeviceEncodingPass.
Supporting pad materialization through the MaterializeDeviceEncodingPass
is left as TODO.

This PR is a step towards #20202

---------

Signed-off-by: Max Dawkins <max.dawkins@gmail.com>
Max191 added a commit that referenced this issue Mar 20, 2025
@Max191
[DataTiling][Codegen] Use iree.encoding.resolver to decide encoding r…
e17fa18 
…esolvers (#20271)

Instead of building the encoding layout attr based on the
`hal.executable.target` attr, the resolver is taken directly from the
`iree.encoding.resolver` attr in the target configuration. We then call
`cloneWithSimplifiedConfig` to attach the needed target information to
the resolver, and we use the cloned layout attr for type conversion.

There was a bug in the `cloneWithSimplifiedConfig` implementation for
CPU, and it was not picking up the `ukernels` information. The bug is
fixed in this PR, since we are now using the `cloneWithSimplifiedConfig`
for the materialization type converter.

closes #20202

---------

Signed-off-by: Max Dawkins <max.dawkins@gmail.com>
Eliasj42 pushed a commit that referenced this issue Mar 24, 2025
@Max191
[DataTiling][GPU] Add layout resolver option to ROCMTarget (#20250)
b5fbdd5 
Add a new flag option to ROCMTarget called
`iree-hip-encoding-layout-resolver`. The option selects between the
different layout resolvers for encoding attributes:
 - `none`: Select the default layout resolver (materialize to nop)
- `pad`: Select the GPUPadLayoutAttr resolver (materialize into padding)
- `data-tiling`: Select the GPUEncodingLayoutAttr resolver (data tiling
materialization)

The `iree-hip-enable-experimental-pad-layout` and
`iree-llvmgpu-experimental-data-tiling` are removed because they are
folded into the new option.

Currently, we run both the `MaterializeDeviceEncodingPass` and the
`MaterializeEncodingIntoPaddingPass` because the padding layout
materialization cannot be handled by the MaterializeDeviceEncodingPass.
Supporting pad materialization through the MaterializeDeviceEncodingPass
is left as TODO.

This PR is a step towards #20202

---------

Signed-off-by: Max Dawkins <max.dawkins@gmail.com>
Signed-off-by: Elias Joseph <eljoseph@amd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@hanhanW hanhanW

@Max191 Max191

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[DataTiling][Codegen] Use iree.encoding.resolver to decide encoding resolvers Max191/iree
4 participants
@hanhanW @pashu123 @jtuyls @Max191
0