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

Do not serialize-deserialize module before calling aie2xclbin #685

Merged
merged 1 commit into from
Aug 21, 2024

Conversation

newling
Copy link
Contributor

@newling newling commented Aug 17, 2024

This PR does a few things, happy to split it up if requested.

  1. Before pulling mlir-aie into iree-amd-aie, it we required to serialize-deserialize to call the aie2xclbin program. But we now just use aie2xclbin as a function, no shell out anymore. So no serialization-deserialization needed.

  2. This PR moves dma-to-npu closer to lower-to-aie pass. I think we'd eventually like to change the lowering of npu instructions from

amdaie dialect -> aie dialect -> npu

to

amdaie dialect  -> npu

, because the amdaie and aie dialects are very similar and this indirection doesn't provide us with anything afaict. Making that change is currently not possible (dma-to-npu must currently run after stateful transform pass), this change is a step in that direction though

Test changes: I removed some CHECKs for aiex.runtime_sequence in tests/samples, because that's now sucked into the LX instructions (sequence of integers). In my mind the tests in tests/samples are only useful to check that compilation doesn't error/crash, so IMO removing CHECK lines there is fine.

  1. General clean-up, for example we don't need aiex-to-standard pass anymore.

@makslevental
Copy link
Collaborator

Currently dma-to-npu runs in aie2xclbin, we shouldn't be running mlir passes there because it's on the other side of the serialization wall.

100% agreed. I recommend you xfail (for a moment) those failing tests to see if this will work e2e.

@newling newling changed the title [WIP] Run dma-to-npu pass earlier Run dma-to-npu pass earlier Aug 19, 2024
@newling newling force-pushed the dma_to_npu_before_aie2xclbin branch 2 times, most recently from 3a8534e to a5b7010 Compare August 19, 2024 21:15
@newling newling changed the title Run dma-to-npu pass earlier Do not serialize-deserialize module before calling aie2xclbin Aug 19, 2024
@newling newling requested a review from makslevental August 20, 2024 00:20
Copy link
Collaborator

@makslevental makslevental left a comment

Choose a reason for hiding this comment

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

Overall looks good but a couple of small questions/nits.

@newling newling force-pushed the dma_to_npu_before_aie2xclbin branch from a5b7010 to fbf482e Compare August 20, 2024 04:48
Copy link
Collaborator

@makslevental makslevental left a comment

Choose a reason for hiding this comment

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

Cool thanks for this change (it's a small but important one). Sorry for the hassle :(

@newling newling force-pushed the dma_to_npu_before_aie2xclbin branch from 47b12ed to cf909f3 Compare August 20, 2024 19:52
@newling newling enabled auto-merge (squash) August 20, 2024 19:54
@makslevental makslevental force-pushed the dma_to_npu_before_aie2xclbin branch from cf909f3 to a7219d9 Compare August 21, 2024 05:49
@newling newling merged commit 57cd0bc into nod-ai:main Aug 21, 2024
3 checks passed
@newling newling deleted the dma_to_npu_before_aie2xclbin branch August 29, 2024 18:26
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.

2 participants