Skip to content

Commit 5c48b3c

Browse files
hunhoffeclaude
andcommitted
Merge origin/main into dma-bd-ssa-operands
Integrates PR #3308 (dynamic lock acquisition for ObjectFIFO): - All test files: use_lock uses new SSA i32 value operand form (arith.constant + %c) - All test files: dma_bd retains this PR's SSA operand format (offset=/len=/sizes=/strides=) - bad_bd_assignments.mlir: includes #3308's new nextBdId overflow test - dynamic_cyclostatic_* test files removed (deleted upstream by #3308) - New dma_channel_pinning/ tests and dynamic_runtime_lock_* tests from #3308 Co-Authored-By: Claude <noreply@anthropic.com>
2 parents 868d7cf + ee23ed1 commit 5c48b3c

325 files changed

Lines changed: 11141 additions & 6841 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CMakeLists.txt

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -344,9 +344,18 @@ endif()
344344

345345

346346
# Last because each of these has its own CMakeLists.txt which reloads/re-finds LLVM, thus resettings globals.
347-
add_subdirectory(programming_examples)
348-
add_subdirectory(programming_guide)
349-
add_subdirectory(test)
347+
# The test/example/tutorial subdirectories' check-* lit targets depend on AIEPythonModules, so they
348+
# require the Python bindings. Gate them behind an option that is forced OFF when the bindings are
349+
# off (mirroring how AIE_ENABLE_BINDINGS_PYTHON itself is defined above), so the compiler can be
350+
# configured and built standalone with a single -DAIE_ENABLE_BINDINGS_PYTHON=OFF.
351+
cmake_dependent_option(AIE_INCLUDE_TESTS_AND_EXAMPLES
352+
"Build the test/example/tutorial subdirectories (require the Python bindings)" ON
353+
"AIE_ENABLE_BINDINGS_PYTHON" OFF)
354+
if(AIE_INCLUDE_TESTS_AND_EXAMPLES)
355+
add_subdirectory(programming_examples)
356+
add_subdirectory(programming_guide)
357+
add_subdirectory(test)
358+
endif()
350359
add_subdirectory(cmake/modules)
351360

352361
get_filename_component(COMPILER_DIRECTORY ${CMAKE_CXX_COMPILER} DIRECTORY)

include/aie/Dialect/AIE/IR/AIEOps.td

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1444,34 +1444,49 @@ def AIE_UseLockOp: AIE_Op<"use_lock", []> {
14441444
Then, the value of the lock is decremented by `value`.
14451445
- Release: In AIE1, set the lock to `value`.
14461446
In AIE2, increment the lock by `value`.
1447+
1448+
The lock value is always given as an `i32` SSA `value` operand. Passes that
1449+
need a compile-time constant obtain it via `getConstantValue()`, which
1450+
expects the operand to be defined by an `arith.constant` and fails
1451+
otherwise.
14471452
}];
14481453

14491454
let arguments = (
14501455
ins Index:$lock,
14511456
LockAction:$action,
1452-
OptionalAttr<AIEI32Attr>:$value,
1457+
I32:$value,
14531458
OptionalAttr<LockBlocking>:$blocking,
14541459
DefaultValuedOptionalAttr<BoolAttr, "true">:$acq_en
14551460
);
14561461

14571462
let assemblyFormat = [{
1458-
`(` $lock `,` $action (`,` $value^)? (`,` $blocking^)? `)` attr-dict
1463+
`(` $lock `,` $action `,` $value (`,` $blocking^)? `)` attr-dict
14591464
}];
14601465

14611466
let hasVerifier = 1;
14621467
let builders = [
14631468
OpBuilder<(ins "mlir::Value":$lock,
14641469
"xilinx::AIE::LockAction":$action,
14651470
"int32_t":$value), [{
1466-
build($_builder, $_state, lock, action, $_builder.getI32IntegerAttr(value), nullptr);
1471+
auto constant = mlir::arith::ConstantOp::create(
1472+
$_builder, $_state.location, $_builder.getI32Type(),
1473+
$_builder.getI32IntegerAttr(value));
1474+
build($_builder, $_state, lock, action, constant, /*blocking=*/nullptr);
1475+
}]>,
1476+
OpBuilder<(ins "mlir::Value":$lock,
1477+
"xilinx::AIE::LockAction":$action,
1478+
"mlir::Value":$value), [{
1479+
build($_builder, $_state, lock, action, value, /*blocking=*/nullptr);
14671480
}]>
14681481
];
14691482

14701483
let extraClassDeclaration = [{
14711484
bool acquire() { return (getAction() == LockAction::Acquire); }
14721485
bool acquireGE() { return (getAction() == LockAction::AcquireGreaterEqual); }
14731486
bool release() { return (getAction() == LockAction::Release); }
1474-
int getLockValue() { return getValue().value_or(1); }
1487+
// Returns the compile-time constant lock value when `value` is defined by
1488+
// an arith.constant; otherwise emits an op error and returns failure.
1489+
mlir::FailureOr<int32_t> getConstantValue();
14751490
int getTimeout() {
14761491
// LockBlocking is an EnumAttr.
14771492
if (auto val = getBlocking())
@@ -1906,6 +1921,11 @@ def AIE_ObjectFifoCreateOp: AIE_Op<"objectfifo", [HasParent<"DeviceOp">, Symbol]
19061921
// aie_stream==2 means enable aie stream ports on producer and consumer tiles
19071922
OptionalAttr<ConfinedAttr<AIEI32Attr, [IntMinValue<0>, IntMaxValue<2>]>>:$aie_stream,
19081923
OptionalAttr<ConfinedAttr<AIEI32Attr, [IntMinValue<0>, IntMaxValue<1>]>>:$aie_stream_port,
1924+
// Pin the DMA channel for an endpoint instead of first-free assignment.
1925+
// prod_dma_channel pins the producer endpoint's channel; cons_dma_channels
1926+
// holds one entry per consumer (-1 = auto-assign that consumer).
1927+
OptionalAttr<ConfinedAttr<AIEI32Attr, [IntMinValue<0>]>>:$prod_dma_channel,
1928+
OptionalAttr<DenseI32ArrayAttr>:$cons_dma_channels,
19091929
InitValuesArrayAttr:$initValues,
19101930
OptionalAttr<BDPadLayoutArrayAttr>:$padDimensions,
19111931
OptionalAttr<AIEI32Attr>:$iter_count

lib/Dialect/AIE/IR/AIEDialect.cpp

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "mlir/Dialect/MemRef/IR/MemRef.h"
1414
#include "mlir/Dialect/Utils/StaticValueUtils.h"
1515
#include "mlir/IR/DialectImplementation.h"
16+
#include "mlir/IR/Matchers.h"
1617
#include "mlir/IR/OpDefinition.h"
1718
#include "mlir/IR/SymbolTable.h"
1819
#include "mlir/Interfaces/FoldInterfaces.h"
@@ -76,7 +77,11 @@ struct AIEDialectFoldInterface : DialectFoldInterface {
7677
// them with a core's identical constants, which would leave the core
7778
// referencing a device-level value that is erased when the core is
7879
// outlined.
79-
return isa<CoreOp, RuntimeSequenceOp>(region->getParentOp());
80+
// - Make sure SSA values for aie.use_lock operands in
81+
// aie.mem/aie.memtile_dma/aie.shim_dma bodies do not get
82+
// hoisted.
83+
return isa<CoreOp, RuntimeSequenceOp, MemOp, MemTileDMAOp, ShimDMAOp>(
84+
region->getParentOp());
8085
}
8186
};
8287

@@ -2790,6 +2795,15 @@ LogicalResult LockOp::verify() {
27902795
return success();
27912796
}
27922797

2798+
// Look up for compile-time constant lock values, if any.
2799+
// Returns std::nullopt if lock value does not reference an `arith.constant`.
2800+
static std::optional<int32_t> getConstantLockValue(UseLockOp op) {
2801+
if (auto constant = op.getValue().getDefiningOp<arith::ConstantOp>())
2802+
if (auto intAttr = llvm::dyn_cast<IntegerAttr>(constant.getValue()))
2803+
return (int32_t)intAttr.getInt();
2804+
return std::nullopt;
2805+
}
2806+
27932807
struct UsesOneLockInDMABlock {
27942808
static LogicalResult verifyTrait(Operation *op) {
27952809
auto *block = op->getBlock();
@@ -2811,16 +2825,21 @@ struct AcquireReleaseOneStateInDMABlock {
28112825
auto *block = op->getBlock();
28122826
int acqValue = -1, relValue = -1;
28132827
for (auto op : block->getOps<UseLockOp>()) {
2828+
// Non-constant lock values cannot be compared here; the passes that
2829+
// require a constant enforce that separately via getConstantValue().
2830+
auto value = getConstantLockValue(op);
2831+
if (!value)
2832+
continue;
28142833
if (op.acquire() || op.acquireGE()) {
2815-
if (acqValue != -1 && acqValue != op.getLockValue()) {
2834+
if (acqValue != -1 && acqValue != *value) {
28162835
return failure();
28172836
}
2818-
acqValue = op.getLockValue();
2837+
acqValue = *value;
28192838
} else if (op.release()) {
2820-
if (relValue != -1 && relValue != op.getLockValue()) {
2839+
if (relValue != -1 && relValue != *value) {
28212840
return failure();
28222841
}
2823-
relValue = op.getLockValue();
2842+
relValue = *value;
28242843
}
28252844
}
28262845
return success();
@@ -2848,6 +2867,15 @@ LogicalResult UseLockOp::verify() {
28482867
return (*this)->emitOpError(
28492868
"AcquireGreaterEqual is not supported in AIE1.");
28502869

2870+
// Locks used inside a DMA/BD block are configured via static register writes
2871+
// and therefore require a compile-time constant value.
2872+
if (HasSomeParent<MemOp, MemTileDMAOp, ShimDMAOp>::verifyTrait(*this)
2873+
.succeeded() &&
2874+
!getConstantLockValue(*this))
2875+
return (*this)->emitOpError(
2876+
"lock value in a DMA/BD block must be a compile-time constant "
2877+
"(defined by an arith.constant).");
2878+
28512879
// Otherwise, AIE.useLock should be inside MemOp, MemTileDMAOp, or
28522880
// ShimDMAOp,
28532881
if (HasSomeParent<MemOp, MemTileDMAOp, ShimDMAOp>::verifyTrait(*this)
@@ -2932,9 +2960,17 @@ static void printTraceRegValue(OpAsmPrinter &printer, Operation *op,
29322960
xilinx::AIE::printTraceEventEnum(printer, value);
29332961
}
29342962

2963+
// Helper to parse a LockBlocking enum keyword.
29352964
#define GET_OP_CLASSES
29362965
#include "aie/Dialect/AIE/IR/AIEOps.cpp.inc"
29372966

2967+
FailureOr<int32_t> UseLockOp::getConstantValue() {
2968+
if (auto value = getConstantLockValue(*this))
2969+
return *value;
2970+
return emitOpError("expected the lock value to be a compile-time constant "
2971+
"(defined by an arith.constant).");
2972+
}
2973+
29382974
TileOp SwitchboxOp::getTileOp() {
29392975
return cast<TileElement>(this->getOperation()).getTileOp();
29402976
}

lib/Dialect/AIE/Transforms/AIECoreToStandard.cpp

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -443,19 +443,19 @@ struct AIEUseLockToStdLowering : OpConversionPattern<UseLockOp> {
443443
return useLock.emitOpError("Could not find the intrinsic function!");
444444

445445
SmallVector<Value, 2> args;
446-
auto lockValue = useLock.getLockValue();
446+
auto i32Ty = IntegerType::get(rewriter.getContext(), 32);
447+
args.push_back(arith::IndexCastOp::create(rewriter, useLock.getLoc(),
448+
i32Ty, useLock.getLock()));
447449

448-
// AIE2 acquire greater equal is encoded as a negative value.
450+
Value value = adaptor.getValue();
451+
// AIE2 acquire-greater-equal is encoded as a negative value, so negate
452+
// it at runtime.
449453
if (useLock.acquireGE()) {
450-
lockValue = -lockValue;
454+
Value zero = arith::ConstantOp::create(
455+
rewriter, useLock.getLoc(), i32Ty, rewriter.getI32IntegerAttr(0));
456+
value = arith::SubIOp::create(rewriter, useLock.getLoc(), zero, value);
451457
}
452-
args.push_back(arith::IndexCastOp::create(
453-
rewriter, useLock.getLoc(),
454-
IntegerType::get(rewriter.getContext(), 32), useLock.getLock()));
455-
args.push_back(
456-
arith::ConstantOp::create(rewriter, useLock.getLoc(),
457-
IntegerType::get(rewriter.getContext(), 32),
458-
rewriter.getI32IntegerAttr(lockValue)));
458+
args.push_back(value);
459459

460460
func::CallOp::create(rewriter, useLock.getLoc(), useLockFunc, args);
461461
}

0 commit comments

Comments
 (0)