Skip to content

Commit 4b53464

Browse files
committed
Fix annotations
Signed-off-by: Julia Koval <c_yuliak@xsightlabs.com>
1 parent ff53252 commit 4b53464

File tree

4 files changed

+50
-5
lines changed

4 files changed

+50
-5
lines changed

include/p4mlir/Dialect/P4HIR/P4HIR_Ops.td

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,8 @@ def CmpOp : P4HIR_Op<"cmp",
540540
def ScopeOp : P4HIR_Op<"scope", [
541541
DeclareOpInterfaceMethods<RegionBranchOpInterface>,
542542
RecursivelySpeculatable,
543-
RecursiveMemoryEffects, AutomaticAllocationScope,
543+
DeclareOpInterfaceMethods<MemoryEffectsOpInterface>,
544+
AutomaticAllocationScope,
544545
NoRegionArguments, Annotated]> {
545546
let summary = "Represents a P4 scope";
546547
let description = [{

lib/Dialect/P4HIR/P4HIR_Ops.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -926,6 +926,28 @@ LogicalResult P4HIR::ScopeOp::verify() {
926926
return success();
927927
}
928928

929+
void P4HIR::ScopeOp::getEffects(
930+
SmallVectorImpl<SideEffects::EffectInstance<MemoryEffects::Effect>> &effects) {
931+
if (auto annotations = getAnnotations()) {
932+
auto attrs = annotations->getValue();
933+
bool isOnlyAtomic = (attrs.size() == 1 && attrs[0].getName() == "atomic");
934+
if (!isOnlyAtomic) {
935+
effects.emplace_back(MemoryEffects::Write::get(), SideEffects::DefaultResource::get());
936+
return;
937+
}
938+
}
939+
940+
for (Region &region : getOperation()->getRegions()) {
941+
for (Block &block : region) {
942+
for (Operation &op : block) {
943+
if (auto opEffects = mlir::getEffectsRecursively(&op)) {
944+
effects.append(opEffects->begin(), opEffects->end());
945+
}
946+
}
947+
}
948+
}
949+
}
950+
929951
LogicalResult P4HIR::ScopeOp::canonicalize(P4HIR::ScopeOp op, PatternRewriter &rewriter) {
930952
// Canonicalize scope: one without variables could be inlined
931953
if (op.getOps<VariableOp>().empty() && op.getScopeRegion().hasOneBlock() &&

lib/Transforms/FlattenCFG.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,7 @@ class ScopeOpFlattening : public mlir::OpRewritePattern<P4HIR::ScopeOp> {
8686
mlir::OpBuilder::InsertionGuard guard(rewriter);
8787
auto loc = scopeOp.getLoc();
8888

89-
// Empty scope: just remove it.
90-
// TODO: Decide if we'd need to do something with annotated scopes
91-
if (scopeOp.isEmpty()) {
89+
if (scopeOp.isEmpty() && !scopeOp.getAnnotations()) {
9290
rewriter.eraseOp(scopeOp);
9391
return mlir::success();
9492
}

test/Transforms/Folds/scope_simplification.mlir

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,12 +68,36 @@ module {
6868

6969
// CHECK-LABEL: @empty_scope_with_annotations_preserved
7070
p4hir.func @empty_scope_with_annotations_preserved(%arg0: !ref_i32i) {
71-
// TODO: Doesn't work(but should)
71+
// Empty scopes with annotations should be preserved
72+
// CHECK: p4hir.scope annotations {name = "test"} {
73+
// CHECK-NEXT: }
74+
// CHECK-NEXT: p4hir.return
7275
p4hir.scope annotations {name = "test"} {
7376
}
7477
p4hir.return
7578
}
7679

80+
// CHECK-LABEL: @empty_scope_with_atomic_removed
81+
p4hir.func @empty_scope_with_atomic_removed(%arg0: !ref_i32i) {
82+
// Empty scopes with only @atomic annotation should be removed
83+
// CHECK-NOT: p4hir.scope
84+
// CHECK: p4hir.return
85+
p4hir.scope annotations {atomic} {
86+
}
87+
p4hir.return
88+
}
89+
90+
// CHECK-LABEL: @empty_scope_with_atomic_and_name_preserved
91+
p4hir.func @empty_scope_with_atomic_and_name_preserved(%arg0: !ref_i32i) {
92+
// Empty scopes with @atomic plus other annotations should be preserved
93+
// CHECK: p4hir.scope annotations {atomic, name = "debug"} {
94+
// CHECK-NEXT: }
95+
// CHECK-NEXT: p4hir.return
96+
p4hir.scope annotations {atomic, name = "debug"} {
97+
}
98+
p4hir.return
99+
}
100+
77101
// CHECK-LABEL: @scope_in_if_else_without_vars
78102
p4hir.func @scope_in_if_else_without_vars(%cond: !p4hir.bool, %arg0: !ref_i32i) {
79103
// Scope without variables/annotations inside if else branch should be inlined

0 commit comments

Comments
 (0)