mlir-aie icon indicating copy to clipboard operation
mlir-aie copied to clipboard

[AIETarget] Support packet stream for ShimDMAs

Open hanchenye opened this issue 3 years ago • 13 comments

This patch sets the packet bit when DMABDPacketOp is found in the ShimDMAOp region.

This patch also adds a 32x32x32 GEMM unit test. Unfortunately, currently this test case doesn't work on board -- the AIE stalls waiting for the shim DMA to send out the packet.

The layout of the design: image

hanchenye avatar Feb 02 '22 16:02 hanchenye

Ready to merge?

stephenneuendorffer avatar Feb 03 '22 23:02 stephenneuendorffer

Ready to merge?

Because it doesn't work on board, I'm not sure whether there's anything wrong..

hanchenye avatar Feb 04 '22 00:02 hanchenye

@jackl-xilinx Can you attempt to debug this?

stephenneuendorffer avatar Apr 19 '22 17:04 stephenneuendorffer

@hanchenye #133 made some modifications to the create packet flow pass for the shimmux. It includes a working test on the board.

jgmelber avatar Jul 11 '22 20:07 jgmelber

Hi @hanchenye. I apologize for not having addressed this PR for so long. I believe we ran into it again with @JinmingZhuang 's design which was fixed in #133 . I'll double check the design you're trying to check in here and see if I can verify it working on a board, then check if this change is still relevant and either merge it or remove it.

jackl-xilinx avatar Aug 18 '22 15:08 jackl-xilinx

@jackl-xilinx I think #133 addresses most of this but does not make the same changes to lib/Targets/AIETargetXAIEV2.cpp

jgmelber avatar Aug 18 '22 15:08 jgmelber

@jackl-xilinx Cool, thanks!

hanchenye avatar Aug 18 '22 17:08 hanchenye

@jgmelber I made a new pull request here #154 which adds this functionality to lib/Targets/AIETargetXAIEV2.cpp.

JinmingZhuang avatar Aug 18 '22 23:08 JinmingZhuang

@hanchenye Has this PR been superseded by #154 ?

jgmelber avatar Nov 01 '22 22:11 jgmelber

What is the status of this PR?

keryell avatar Jul 19 '23 22:07 keryell

Someone needs to look at this design again. My guess is that it will work, now that we've fixed some unrelated backend issues.

stephenneuendorffer avatar Jul 19 '23 22:07 stephenneuendorffer

@stephenneuendorffer this is already implemented/merged/whatever right? I can see similar (same) lines in some places in the the moved/adjusted target files.

makslevental avatar Dec 01 '23 03:12 makslevental

@stephenneuendorffer this is already implemented/merged/whatever right? I can see similar (same) lines in some places in the the moved/adjusted target files.

I think so.. I think it's mainly a matter of looking at the test designs. It's unclear to me whether this is well tested at the moment.

stephenneuendorffer avatar Dec 01 '23 04:12 stephenneuendorffer