Skip to content

Conversation

@amd-songpiao
Copy link

Do not create transpose ops with non-default layout in DotDecomposer.

The DotDecomposer pass runs ahead of layout assignment. Introducing non-default layouts at this stage causes complications for subsequent passes, in particular the DotMerger pass.

It also prevents generating fat fused triton kernels with output_tiles [1, 1], which leads to register spilling.

related issue - https://github.com/ROCm/frameworks-internal/issues/13595
related PR - #428

…otDecomposer`.

The `DotDecomposer` pass runs ahead of layout assignment. Introducing non-default layouts at this stage causes complications for subsequent passes, in particular the `DotMerger` pass.

It also prevents generating fat fused triton kernels with output_tiles [1, 1], which leads to register spilling.

PiperOrigin-RevId: 824476578
Copy link
Collaborator

@i-chaochen i-chaochen left a comment

Choose a reason for hiding this comment

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

after we have this, we don't need to disable trion_softmax anymore? 3febc29#diff-778936bfc8e1f6ad8322e9795a405576e306dcd8f2479dc3d29b0b9a6f51a832R551

Just to be aware of that jax is already released 0.7.1 and they are targeting 0.8 now. Could you check this whether in 0.8?

@amd-songpiao
Copy link
Author

after we have this, we don't need to disable trion_softmax anymore? 3febc29#diff-778936bfc8e1f6ad8322e9795a405576e306dcd8f2479dc3d29b0b9a6f51a832R551

Just to be aware of that jax is already released 0.7.1 and they are targeting 0.8 now. Could you check this whether in 0.8?

trion_softmax flag is by default enabled in 0.7.1. In upstream, there is no such flag, triton_softmax is always enabled.
I just checked, the commit is also valid for v0.8.0 branch, as v0.8.0 is forked on 15 Oct, the commit is from 27 Oct.

@i-chaochen
Copy link
Collaborator

Thanks! then please backport it to 0.8 as well.

@amd-songpiao
Copy link
Author

please use the PR #432.

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.

4 participants