-
Notifications
You must be signed in to change notification settings - Fork 86
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
Fixing up PLIO compiler support and creating an example #1623
Changes from all commits
e280dec
750aa09
2781398
ef1fae0
c81b751
8973628
49bc24c
965403a
a29234b
ea0aef1
c5f2f29
3aad40c
a915fb8
16dd328
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -104,8 +104,8 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
SwitchboxOp swOp = analyzer.getSwitchbox(rewriter, curr.col, curr.row); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
int shimCh = srcChannel; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// TODO: must reserve N3, N7, S2, S3 for DMA connections | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (curr == srcSB && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
analyzer.getTile(rewriter, srcSB.col, srcSB.row).isShimNOCTile()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (curr == srcSB && analyzer.getTile(rewriter, srcSB.col, srcSB.row) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.isShimNOCorPLTile()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// shim DMAs at start of flows | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (srcBundle == WireBundle::DMA) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
shimCh = srcChannel == 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -125,13 +125,10 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
srcBundle, srcChannel, WireBundle::North, shimCh); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} else if (srcBundle == | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
WireBundle::PLIO) { // PLIO at start of flows with mux | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (srcChannel == 2 || srcChannel == 3 || srcChannel == 6 || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
srcChannel == 7) { // Only some PLIO requrie mux | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ShimMuxOp shimMuxOp = analyzer.getShimMux(rewriter, srcSB.col); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
addConnection( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
rewriter, cast<Interconnect>(shimMuxOp.getOperation()), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
flowOp, srcBundle, srcChannel, WireBundle::North, shimCh); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ShimMuxOp shimMuxOp = analyzer.getShimMux(rewriter, srcSB.col); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's unclear to me what the point of this change is, but I don't understand the original code necessarily. Is this related to the fact that you only 'support' channel 0 and 1 in the code below? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know how correct the previous code was, but in the pass here: mlir-aie/lib/Targets/AIETargetXAIEV2.cpp Lines 709 to 767 in d08090b
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, I get it. I think the original code here was conceptually incorrect. The intention is/was to be explicit about all the connections that exist even the non-programmable ones. Conceptually the 'shimMux' op is intended to represent all of the connectivity in the shim south of the stream switch. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
addConnection(rewriter, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
cast<Interconnect>(shimMuxOp.getOperation()), flowOp, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
srcBundle, srcChannel, WireBundle::North, shimCh); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for (const auto &[bundle, channel] : setting.dsts) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -146,7 +143,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
bundle == WireBundle::NOC)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
shimCh = channel; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (analyzer.getTile(rewriter, curr.col, curr.row) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.isShimNOCTile()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.isShimNOCorPLTile()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// shim DMAs at end of flows | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (bundle == WireBundle::DMA) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
shimCh = channel == 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -162,8 +159,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
addConnection( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
rewriter, cast<Interconnect>(shimMuxOp.getOperation()), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
flowOp, WireBundle::North, shimCh, bundle, channel); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} else if (channel >= | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
2) { // must be PLIO...only PLIO >= 2 require mux | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} else if (bundle == WireBundle::PLIO) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ShimMuxOp shimMuxOp = analyzer.getShimMux(rewriter, curr.col); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
addConnection( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
rewriter, cast<Interconnect>(shimMuxOp.getOperation()), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -416,7 +412,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
keepPktHeaderAttr[{destTile, destPort}] = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
StringAttr::get(Op.getContext(), "true"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Switchbox srcSB = {srcCoords.col, srcCoords.row}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (PathEndPoint srcPoint = {srcSB, srcPort}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Check warning on line 415 in lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp GitHub Actions / ubuntu-22.04 gcc assert=OFF rtti=OFF
Check warning on line 415 in lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp GitHub Actions / ubuntu-22.04 gcc assert=OFF rtti=OFF
Check warning on line 415 in lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp GitHub Actions / ubuntu-22.04 gcc assert=ON rtti=ON
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
!analyzer.processedFlows[srcPoint]) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
SwitchSettings settings = analyzer.flowSolutions[srcPoint]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// add connections for all the Switchboxes in SwitchSettings | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
# This file is licensed under the Apache License v2.0 with LLVM Exceptions. | ||
# See https://llvm.org/LICENSE.txt for license information. | ||
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
# | ||
# (c) Copyright 2023 Advanced Micro Devices, Inc. | ||
|
||
# parameters | ||
# -DBOOST_ROOT: Path to Boost install | ||
# -DXRT_INC_DIR: Full path to src/runtime_src/core/include in XRT cloned repo | ||
# -DXRT_LIB_DIR: Path to xrt_coreutil.lib | ||
# -DTARGET_NAME: Target name to be built | ||
|
||
# cmake needs this line | ||
cmake_minimum_required(VERSION 3.1) | ||
|
||
set(CMAKE_CXX_STANDARD 23) | ||
set(CMAKE_CXX_STANDARD_REQUIRED YES) | ||
|
||
find_program(WSL NAMES powershell.exe) | ||
|
||
if (NOT WSL) | ||
set(CMAKE_C_COMPILER gcc-13) | ||
set(CMAKE_CXX_COMPILER g++-13) | ||
set(BOOST_ROOT /usr/include/boost CACHE STRING "Path to Boost install") | ||
set(XRT_INC_DIR /opt/xilinx/xrt/include CACHE STRING "Path to XRT cloned repo") | ||
set(XRT_LIB_DIR /opt/xilinx/xrt/lib CACHE STRING "Path to xrt_coreutil.lib") | ||
else() | ||
set(BOOST_ROOT C:/Technical/thirdParty/boost_1_83_0 CACHE STRING "Path to Boost install") | ||
set(XRT_INC_DIR C:/Technical/XRT/src/runtime_src/core/include CACHE STRING "Path to XRT cloned repo") | ||
set(XRT_LIB_DIR C:/Technical/xrtNPUfromDLL CACHE STRING "Path to xrt_coreutil.lib") | ||
endif() | ||
|
||
set(TARGET_NAME test CACHE STRING "Target to be built") | ||
|
||
SET (ProjectName proj_${TARGET_NAME}) | ||
SET (currentTarget ${TARGET_NAME}) | ||
|
||
if ( WSL ) | ||
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY_RELEASE ${CMAKE_BINARY_DIR}) | ||
endif () | ||
|
||
project(${ProjectName}) | ||
|
||
# Find packages | ||
find_package(Boost REQUIRED) | ||
|
||
add_executable(${currentTarget} | ||
${CMAKE_CURRENT_SOURCE_DIR}/../../../runtime_lib/test_lib/test_utils.cpp | ||
test.cpp | ||
) | ||
|
||
target_compile_definitions(${currentTarget} PUBLIC DISABLE_ABI_CHECK=1) | ||
|
||
target_include_directories (${currentTarget} PUBLIC | ||
${XRT_INC_DIR} | ||
${Boost_INCLUDE_DIRS} | ||
${CMAKE_CURRENT_SOURCE_DIR}/../../../runtime_lib/test_lib | ||
) | ||
|
||
target_link_directories(${currentTarget} PUBLIC | ||
${XRT_LIB_DIR} | ||
${Boost_LIBRARY_DIRS} | ||
) | ||
|
||
if (NOT WSL) | ||
target_link_libraries(${currentTarget} PUBLIC | ||
xrt_coreutil | ||
boost_program_options | ||
boost_filesystem | ||
) | ||
else() | ||
target_link_libraries(${currentTarget} PUBLIC | ||
xrt_coreutil | ||
) | ||
endif() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
##===- Makefile -----------------------------------------------------------===## | ||
# | ||
# This file licensed under the Apache License v2.0 with LLVM Exceptions. | ||
# See https://llvm.org/LICENSE.txt for license information. | ||
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
# | ||
# Copyright (C) 2024, Advanced Micro Devices, Inc. | ||
# | ||
##===----------------------------------------------------------------------===## | ||
|
||
srcdir := $(shell dirname $(realpath $(firstword $(MAKEFILE_LIST)))) | ||
|
||
include ${srcdir}/../../makefile-common | ||
|
||
targetname = passThroughDMAs | ||
LENGTH ?= 1024 | ||
|
||
all: input output | ||
|
||
build/aie-input-plio.mlir: ${srcdir}/aie2-input-plio.py | ||
mkdir -p ${@D} | ||
python3 $< ${LENGTH} > $@ | ||
|
||
build/aie-output-plio.mlir: ${srcdir}/aie2-output-plio.py | ||
mkdir -p ${@D} | ||
python3 $< ${LENGTH} > $@ | ||
|
||
input: build/aie-input-plio.mlir | ||
aiecc.py --link_against_hsa --host-target=x86_64-amd-linux-gnu build/aie-input-plio.mlir \ | ||
-I${srcdir}/../../../install/runtime_lib/x86_64-hsa/test_lib/include \ | ||
-L/lib/x86_64-linux-gnu/ \ | ||
${srcdir}/test_vck5000.cpp \ | ||
${srcdir}/../../../install/runtime_lib/x86_64-hsa/test_lib/src/test_library.cpp \ | ||
-Wl,--whole-archive -Wl,--no-whole-archive -lstdc++ -ldl -lelf -o input.elf | ||
|
||
output: build/aie-output-plio.mlir | ||
aiecc.py --link_against_hsa --host-target=x86_64-amd-linux-gnu build/aie-output-plio.mlir \ | ||
-I${srcdir}/../../../install/runtime_lib/x86_64-hsa/test_lib/include \ | ||
-L/lib/x86_64-linux-gnu/ \ | ||
${srcdir}/test_vck5000.cpp \ | ||
${srcdir}/../../../install/runtime_lib/x86_64-hsa/test_lib/src/test_library.cpp \ | ||
-Wl,--whole-archive -Wl,--no-whole-archive -lstdc++ -ldl -lelf -o output.elf | ||
|
||
run_vck5000: | ||
test.elf | ||
|
||
clean: | ||
rm -rf build aie-output-plio.mlir.prj aie-input-plio.mlir.prj core_* input.elf output.elf |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
<!---//===- README.md --------------------------*- Markdown -*-===// | ||
// | ||
// This file is licensed under the Apache License v2.0 with LLVM Exceptions. | ||
// See https://llvm.org/LICENSE.txt for license information. | ||
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
// | ||
// Copyright (C) 2024, Advanced Micro Devices, Inc. | ||
// | ||
//===----------------------------------------------------------------------===//--> | ||
|
||
# <ins>Passthrough DMAs with PLIO</ins> | ||
|
||
This reference design can be run on the VCK5000 Versal device. This design leverages the same data movement pattern as the [Passthrough DMAs](../passthrough-dmas) example design but it uses a soft DMA. Please see the [platforms repo](https://github.com/Xilinx/ROCm-air-platforms) for more information on how the programmable logic is integrated with the AIEs. This is meant to be an illustrative example to highlight how to integrate PL designs with AIE designs programmed using mlir-aie. | ||
|
||
In the platform, tile (26, 0) has PLIO connected to a DMA implemented in the programmable logic. There are two designs, `aie2-input-plio.py` uses the soft DMA to push data from DRAM into the AIEs, wheras `aie2-output-plio.py` uses the soft DMA to receive data from the AIEs and push it to DRAM. The soft DMA is programmed using the same mechanism as the ShimDMAs. | ||
|
||
In the [design](./aie2.py) data is brought from external memory to `ComputeTile2` and back, without modification from the tile, by using an implicit copy via the compute tile's Data Movement Accelerator (DMA). The data is read from and written to external memory through the Shim tile (`col`, 0). | ||
|
||
The implicit copy is performed using the `object_fifo_link` operation that specifies how input data arriving via `of_in` should be sent further via `of_out` by specifically leveraging the compute tile's DMA. This operation and its functionality are described in more depth in [Section-2b](../../../programming_guide/section-2/section-2b/03_Link_Distribute_Join/README.md#object-fifo-link) of the programming guide. | ||
|
||
|
||
To compile and run the design for VCK5000: | ||
``` | ||
make all | ||
./output.elf // To run the kernel which outputs over PLIO | ||
./input.elf // To run the kernel which inputs over PLIO | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems a bit odd to me.. Can't the objectFifo figure out that it's connected to PLIO based on the shimDMAAllocation op? Could I have an objectFifo connecting 2 PLIOs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am using this as a way for the user to specify whether the corresponding shim tile(s) of an object FIFO should be connected to GMIO or PLIO. Here is an example: https://github.com/Xilinx/mlir-aie/blob/plio-rebase/programming_examples/basic/passthrough_dmas_plio/aie2-input-plio.py#L49.
We do also need to encode this in the shimDMAAllocationOP but that is later in the compilation, and I believe usually not exposed via the IRON python bindings.
I believe an object FIFO could connect 2 PLIOs but have never tested it. It can connect two GMIOs, correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
shimDMAAllocationOp
is currently being generated by the AIEObjectFifoStatefulTransform and as the object FIFO also abstracts theAIEFlowOps
between tiles I think it can be confusing to know where to add new attributes to guide the configuration. A cleaner solution might be to "allow"shimDMAAllocationOps
to exist before the object FIFO lowering pass and have the lowering pass check for them and either: adopt whatever configuration they specify for the shim tile (including the channel index), or completing any missing information if it was not provided (following the same generation it currently uses). Another possibility would be to rely on the currentAIEFlowOp
in the same manner as above for a more general solution that would allow users to specify the ports they want for any tile not just the shim tile.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either works for me the only important thing to me is that this is user facing. That being said, that change in allowing
shimDMAAllocationOps
to be before the object FIFO lowering pass sounds like it could require some refactoring. If it is non-trivial, should that be a separate PR? If so, I can create an issue so we can track it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we go this route, it will indeed require a bit of refactoring and I do think it will be better to have it in a separate PR that this PR would then build on. Let's discuss further and see if any of those solutions would be preferred.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 because it makes things more composable