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

-aie-lower-memcpy generates illegal AIE.mem operation

Open hanchenye opened this issue 3 years ago • 2 comments

In test/create-cores/test_dma1.mlir, -aie-lower-memcpy convert

  AIE.memcpy @token0(1, 2) (%t11 : <%buf0, 0, 256>, %t22 : <%buf1, 0, 256>) : (memref<256xi32>, memref<256xi32>)
  AIE.memcpy @token1(1, 2) (%t11 : <%buf0, 0, 256>, %t33 : <%buf2, 0, 256>) : (memref<256xi32>, memref<256xi32>)

to (only shows the %t11 side)

  %2 = AIE.mem(%0) {
    %15 = AIE.dmaStart(MM2S0, ^bb1, ^bb4)
    ^bb1:
      AIE.useToken @token0(Acquire, 1)
      AIE.dmaBd(<%1 : memref<256xi32>, 0, 256>, 0)
      AIE.useToken @token0(Release, 2)
      br ^bb4
    ^bb2:
    %16 = AIE.dmaStart(MM2S0, ^bb3, ^bb4)
    ^bb3:
      AIE.useToken @token1(Acquire, 1)
      AIE.dmaBd(<%1 : memref<256xi32>, 0, 256>, 0)
      AIE.useToken @token1(Release, 2)
      br ^bb4
    ^bb4:
      AIE.end
  }

It seems there are two issues here:

  1. ^bb2 is unachievable. I think if these two DMAs are chained together, %15 = AIE.dmaStart(MM2S0, ^bb1, ^bb4) should point to ^bb2 rather than ^bb4?
  2. Reuse of MM2S0 channel is marked as illegal in the verification. Who is wrong?

Two related question about the rationale of AIE.mem:

  1. I noticed there are two DMA channels available, if these two DMAs can be mapped to two channels, should the IR looks like this?
  %2 = AIE.mem(%0) {
    %15 = AIE.dmaStart(MM2S0, ^bb1, ^bb3)
    %16 = AIE.dmaStart(MM2S1, ^bb2, ^bb3)
    ^bb1:
      AIE.useToken @token0(Acquire, 1)
      AIE.dmaBd(<%1 : memref<256xi32>, 0, 256>, 0)
      AIE.useToken @token0(Release, 2)
      br ^bb1
    ^bb2:
      AIE.useToken @token1(Acquire, 1)
      AIE.dmaBd(<%1 : memref<256xi32>, 0, 256>, 0)
      AIE.useToken @token1(Release, 2)
      br ^bb2
    ^bb3:
      AIE.end
  }
  1. What is the semantics of the terminator of "bdBlock"? Could it point to a "dmaBlock"? Because I feel if so the semantics is kind of overlapping with the chain successor of AIE.dmaStart :thinking:

I'll be happy to update and fix the -aie-lower-memcpy pass once I have a better understanding on the semantics of AIE.mem!

Hanchen

hanchenye avatar Oct 17 '21 02:10 hanchenye

Indeed, it looks like the pass assumes there is only 1 memcpy

stephenneuendorffer avatar Oct 17 '21 03:10 stephenneuendorffer

Your propose code is almost right. dmaStart is an MLIR terminator, so it has to be more like:

  %2 = AIE.mem(%0) {
    %15 = AIE.dmaStart(MM2S0, ^bb1, ^bb2)
    ^bb2:
    %16 = AIE.dmaStart(MM2S1, ^bb3, ^bb4)
    ^bb1:
      AIE.useToken @token0(Acquire, 1)
      AIE.dmaBd(<%1 : memref<256xi32>, 0, 256>, 0)
      AIE.useToken @token0(Release, 2)
      br ^bb1
    ^bb3:
      AIE.useToken @token1(Acquire, 1)
      AIE.dmaBd(<%1 : memref<256xi32>, 0, 256>, 0)
      AIE.useToken @token1(Release, 2)
      br ^bb2
    ^bb4:
      AIE.end
  }

stephenneuendorffer avatar Oct 17 '21 03:10 stephenneuendorffer