Forward source locations across AIE/AIEX transform passes#3105
Open
fifield wants to merge 5 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves debuggability of synthesized MLIR by propagating meaningful Locations (typically the source op’s loc, or an appropriate parent op’s loc) through AIE/AIEX transform and lowering passes, replacing many builder.getUnknownLoc() uses. It also adds lit tests to guard against regressions in location forwarding.
Changes:
- Replaced
getUnknownLoc()at many op-creation sites across AIE/AIEX passes with source-anchored locations (e.g.,op.getLoc(),device.getLoc(), etc.). - Threaded
Locationparameters through a few internal helper functions to avoid losing loc information. - Added focused
loc_preservation.mlirlit tests for several passes to ensure generated ops do not default toloc(unknown).
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/objectFifo-stateful-transform/base/loc_preservation.mlir | New lit test asserting objectfifo stateful transform preserves user loc onto synthesized buffers/locks. |
| test/lower-to-standard/loc_preservation.mlir | New lit test asserting standard lowering preserves loc onto lowered calls/globals. |
| test/herd-routing/loc_preservation.mlir | New lit test asserting herd-routing synthesized ops preserve HerdOp loc (currently checked via aie.connect). |
| test/create-flows/loc_preservation.mlir | New lit test asserting create-pathfinder-flows preserves FlowOp/tile loc onto synthesized connect/wire. |
| test/create-cores/loc_preservation.mlir | New lit test asserting create-cores + lower-memcpy preserve user locs onto representative synthesized ops. |
| test/Conversion/AIEToConfiguration/loc_preservation.mlir | New lit test asserting convert-aie-to-control-packets preserves DeviceOp loc onto synthesized ops (but currently misses an explicit runtime_sequence loc check). |
| lib/Dialect/AIEX/Transforms/AIELowerMulticast.cpp | Flow ops created during multicast lowering now use source op location. |
| lib/Dialect/AIEX/Transforms/AIELowerMemcpy.cpp | DMA/BD/token/flow op creation now uses the memcpy op location (threaded through helper). |
| lib/Dialect/AIEX/Transforms/AIEHerdRouting.cpp | Herd-routing synthesized iter/select/switchbox/connect ops now use the herd op location. |
| lib/Dialect/AIEX/Transforms/AIEExpandLoadPdi.cpp | Empty device creation now uses NpuLoadPdiOp location rather than unknown. |
| lib/Dialect/AIEX/Transforms/AIECtrlPacketToDma.cpp | Batched DMA memcpy emission now uses the device-derived loc instead of unknown. |
| lib/Dialect/AIEX/Transforms/AIECreateLocks.cpp | Created locks now use the shared tile’s location. |
| lib/Dialect/AIEX/Transforms/AIECreateCores.cpp | Created tile/buffer/mem/core/end ops now use the originating call op location. |
| lib/Dialect/AIEX/Transforms/AIECreateBroadcastPacket.cpp | Broadcast packet expansion ops now use source op locations (broadcastpacket/bpdest). |
| lib/Dialect/AIE/Transforms/AIEObjectFifoStatefulTransform.cpp | Synthesized fifo elements, DMA blocks, globals, packet/flow ops now preserve source op locations (threaded into helpers). |
| lib/Dialect/AIE/Transforms/AIEObjectFifoRegisterProcess.cpp | Register-process generated scf/objfifo/call/release ops now use the register op location. |
| lib/Dialect/AIE/Transforms/AIELowerCascadeFlows.cpp | Cascade configuration now uses tile location instead of unknown. |
| lib/Dialect/AIE/Transforms/AIELocalizeLocks.cpp | Local lock ID constants now use the lock’s location instead of unknown. |
| lib/Dialect/AIE/Transforms/AIEGenerateColumnControlOverlay.cpp | Generated packet flow + shim dma allocation ops now use tile-derived locations (threaded into helper). |
| lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp | Switchbox terminators, rules, connects, wires now use FlowOp/tile-derived locations. |
| lib/Dialect/AIE/Transforms/AIECoreToStandard.cpp | Lowered func calls/globals/returns now use source op locations instead of unknown. |
| lib/Dialect/AIE/Transforms/AIECanonicalizeDevice.cpp | Canonicalized device creation now uses moduleOp.getLoc() rather than unknown. |
| lib/Dialect/AIE/IR/AIEDialect.cpp | TileOp::getOrCreate now uses device.getLoc() for created tiles. |
| lib/Conversion/AIEToConfiguration/AIEToConfiguration.cpp | Transaction/control-packet emission now threads and uses a device-derived location rather than unknown. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This was referenced May 29, 2026
Replace builder.getUnknownLoc() with the source op's location at op- creation sites across the AIE and AIEX transform passes, so that ops synthesized during lowering inherit a useful Location instead of "unknown". This is purely mechanical: every replacement uses the loc of the op being lowered (or a contextually-appropriate parent op such as the surrounding tile, device, or originating flow). Passes updated: AIEObjectFifoStatefulTransform AIECreateBroadcastPacket AIECoreToStandard AIECreateCores AIECreatePathFindFlows AIECreateLocks AIEObjectFifoRegisterProcess AIECtrlPacketToDma AIEGenerateColumnControlOverlay AIEExpandLoadPdi AIECanonicalizeDevice AIEHerdRouting AIELocalizeLocks AIELowerMemcpy AIELowerCascadeFlows AIELowerMulticast AIEDialect (TileOp helper) AIEToConfiguration The few remaining getUnknownLoc() sites (declareAIEIntrinsics, DynamicTileAnalysis helpers, parsed-binary ModuleOp creation) have no source op to attribute and are left alone intentionally. Adds one focused loc_preservation.mlir lit test per converted pass to guard against regression: test/Conversion/AIEToConfiguration/loc_preservation.mlir test/create-cores/loc_preservation.mlir test/create-flows/loc_preservation.mlir test/herd-routing/loc_preservation.mlir test/lower-to-standard/loc_preservation.mlir test/objectFifo-stateful-transform/base/loc_preservation.mlir
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace builder.getUnknownLoc() with the source op's location at op- creation sites across the AIE and AIEX transform passes, so that ops synthesized during lowering inherit a useful Location instead of "unknown". This is purely mechanical: every replacement uses the loc of the op being lowered (or a contextually-appropriate parent op such as the surrounding tile, device, or originating flow).
Passes updated:
AIEObjectFifoStatefulTransform AIECreateBroadcastPacket
AIECoreToStandard AIECreateCores
AIECreatePathFindFlows AIECreateLocks
AIEObjectFifoRegisterProcess AIECtrlPacketToDma
AIEGenerateColumnControlOverlay AIEExpandLoadPdi
AIECanonicalizeDevice AIEHerdRouting
AIELocalizeLocks AIELowerMemcpy
AIELowerCascadeFlows AIELowerMulticast
AIEDialect (TileOp helper) AIEToConfiguration
The few remaining getUnknownLoc() sites (declareAIEIntrinsics, DynamicTileAnalysis helpers, parsed-binary ModuleOp creation) have no source op to attribute and are left alone intentionally.
Adds one focused loc_preservation.mlir lit test per converted pass to guard against regression:
test/Conversion/AIEToConfiguration/loc_preservation.mlir
test/create-cores/loc_preservation.mlir
test/create-flows/loc_preservation.mlir
test/herd-routing/loc_preservation.mlir
test/lower-to-standard/loc_preservation.mlir
test/objectFifo-stateful-transform/base/loc_preservation.mlir