-
Notifications
You must be signed in to change notification settings - Fork 24
Add lowering support for static_assert #326
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
|
|
||
| #include "llvm/ADT/StringSwitch.h" | ||
| #include "llvm/Support/LogicalResult.h" | ||
| #include "mlir/IR/Matchers.h" | ||
| #include "mlir/IR/SymbolTable.h" | ||
| #include "mlir/IR/Types.h" | ||
| #include "mlir/Interfaces/FunctionInterfaces.h" | ||
|
|
@@ -84,6 +85,33 @@ struct CallOpConversionPattern : public OpConversionPattern<P4HIR::CallOp> { | |
| operands.getArgOperands()); | ||
| return success(); | ||
| } | ||
| // Handle static_assert: lower to P4CoreLib::StaticAssertOp | ||
| if (callee.getLeafReference().getValue().starts_with("static_assert")) { | ||
|
Collaborator
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. This is an insufficient check (e.g. there could be a user named function So you should get the overload_set and then check if it's |
||
| auto args = operands.getArgOperands(); | ||
|
|
||
| if (args.empty()) | ||
| return op.emitError("static_assert requires a condition"); | ||
|
|
||
| Value cond = args[0]; | ||
| StringAttr message = nullptr; | ||
| if (args.size() == 2) { | ||
| if (auto constOp = args[1].getDefiningOp<P4HIR::ConstOp>()) { | ||
| message = llvm::dyn_cast<StringAttr>(constOp.getValue()); | ||
| } | ||
| if (!message) { | ||
|
Collaborator
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. Is this allowed to happen? (i.e. a non constant string argument). |
||
| op.emitWarning("static_assert message is not a compile-time constant; ignoring"); | ||
| } | ||
|
|
||
|
Collaborator
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. Remove empty line, plus formatting above is off, have you run clang-format? |
||
| } | ||
|
|
||
| rewriter.replaceOpWithNewOp<P4CoreLib::StaticAssertOp>( | ||
| op, | ||
| op.getResult().getType(), | ||
| cond, | ||
| message | ||
| ); | ||
| return success(); | ||
| } | ||
|
|
||
| return failure(); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| #include "p4mlir/Dialect/P4CoreLib/P4CoreLib_Ops.h" | ||
|
|
||
| #include "p4mlir/Dialect/P4CoreLib/P4CoreLib_Dialect.h" | ||
| #include "p4mlir/Dialect/P4HIR/P4HIR_Attrs.h" | ||
| #include "p4mlir/Dialect/P4HIR/P4HIR_Dialect.h" | ||
|
|
||
| using namespace mlir; | ||
|
|
@@ -14,6 +15,27 @@ void P4CoreLib::PacketLookAheadOp::getAsmResultNames(mlir::OpAsmSetValueNameFn s | |
| setNameFn(getResult(), "lookahead"); | ||
| } | ||
|
|
||
| OpFoldResult P4CoreLib::StaticAssertOp::fold(FoldAdaptor adaptor) { | ||
|
Collaborator
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'd say we shouldn't have this folding pattern. It basically implements half of the pass, and creates redundancy while not providing any clear benefit as we'd have to run the pass anyway. Was this intended to be an optimization? |
||
| // This fold handles the true case early during canonicalization, without | ||
| // needing the full EvaluateStaticAssertPass pipeline. False assertions are | ||
| // intentionally left unhandled here so the pass can emit proper error messages. | ||
| auto condAttr = adaptor.getCond(); | ||
| if (!condAttr) | ||
| return {}; // Condition not constant yet | ||
|
|
||
| auto boolAttr = llvm::dyn_cast<P4HIR::BoolAttr>(condAttr); | ||
| if (!boolAttr) | ||
| return {}; // Not a boolean | ||
|
|
||
| if (boolAttr.getValue()) { | ||
| // Assertion passed -> fold to constant true | ||
| return P4HIR::BoolAttr::get(getContext(), true); | ||
| } | ||
|
|
||
| // Assertion failed -> seperate pass will handle the error | ||
| return {}; | ||
| } | ||
|
|
||
| void P4CoreLib::P4CoreLibDialect::initialize() { | ||
| registerTypes(); | ||
| addOperations< | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| #include "p4mlir/Transforms/Passes.h" | ||
|
|
||
| #include "mlir/IR/PatternMatch.h" | ||
| #include "mlir/Pass/Pass.h" | ||
| #include "p4mlir/Dialect/P4CoreLib/P4CoreLib_Dialect.h" | ||
| #include "p4mlir/Dialect/P4CoreLib/P4CoreLib_Ops.h" | ||
| #include "p4mlir/Dialect/P4HIR/P4HIR_Attrs.h" | ||
| #include "p4mlir/Dialect/P4HIR/P4HIR_Dialect.h" | ||
| #include "p4mlir/Dialect/P4HIR/P4HIR_Ops.h" | ||
|
|
||
| using namespace mlir; | ||
|
|
||
| namespace P4::P4MLIR { | ||
|
|
||
| #define GEN_PASS_DEF_EVALUATESTATICASSERT | ||
| #include "p4mlir/Transforms/Passes.cpp.inc" | ||
|
|
||
| namespace { | ||
|
|
||
| struct EvaluateStaticAssertPass | ||
| : public impl::EvaluateStaticAssertBase<EvaluateStaticAssertPass> { | ||
|
|
||
| void runOnOperation() override { | ||
| auto moduleOp = getOperation(); | ||
| bool hasError = false; | ||
|
|
||
| SmallVector<P4CoreLib::StaticAssertOp> assertOps; | ||
| moduleOp->walk([&](P4CoreLib::StaticAssertOp op) { | ||
| assertOps.push_back(op); | ||
| }); | ||
|
|
||
| for (auto op : assertOps) { | ||
| Value cond = op.getCond(); | ||
|
Collaborator
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. No need for single use variable here, just do |
||
|
|
||
| auto constOp = cond.getDefiningOp<P4HIR::ConstOp>(); | ||
| if (!constOp) | ||
| continue; | ||
|
|
||
| auto boolAttr = llvm::dyn_cast<P4HIR::BoolAttr>(constOp.getValue()); | ||
|
Collaborator
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. Since cond is BooleanType, let's use llvm::cast here and remove |
||
| if (!boolAttr) | ||
| continue; | ||
|
|
||
| if (boolAttr.getValue()) { | ||
| OpBuilder builder(op); | ||
| auto trueAttr = P4HIR::BoolAttr::get(op.getContext(), true); | ||
| auto replacement = builder.create<P4HIR::ConstOp>(op.getLoc(), trueAttr); | ||
| op.replaceAllUsesWith(replacement.getResult()); | ||
| op.erase(); | ||
| } else { | ||
| if (auto msg = op.getMessage()) | ||
| op.emitError() << "static assertion failed: " << msg.value(); | ||
| else | ||
| op.emitError("static assertion failed"); | ||
| hasError = true; | ||
| } | ||
| } | ||
|
|
||
| if (hasError) | ||
| signalPassFailure(); | ||
| } | ||
| }; | ||
|
|
||
| } // namespace | ||
|
|
||
| std::unique_ptr<Pass> createEvaluateStaticAssertPass() { | ||
| return std::make_unique<EvaluateStaticAssertPass>(); | ||
| } | ||
|
|
||
| } // namespace P4::P4MLIR | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| // RUN: p4mlir-opt %s --lower-to-p4corelib | FileCheck %s | ||
|
|
||
| // Test lowering of static_assert calls to p4corelib.static_assert op | ||
|
|
||
| !string = !p4hir.string | ||
| #true = #p4hir.bool<true> : !p4hir.bool | ||
| #false = #p4hir.bool<false> : !p4hir.bool | ||
| #undir = #p4hir<dir undir> | ||
|
|
||
| module { | ||
| p4hir.func @static_assert_0(!p4hir.bool {p4hir.dir = #undir, p4hir.param_name = "check"}, !string {p4hir.dir = #undir, p4hir.param_name = "message"}) -> !p4hir.bool annotations {corelib} | ||
| p4hir.func @static_assert_1(!p4hir.bool {p4hir.dir = #undir, p4hir.param_name = "check"}) -> !p4hir.bool annotations {corelib} | ||
|
Collaborator
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. Following the advice at the top, these must be in a overload_set, as it would be generated by translating the |
||
|
|
||
| // Test: static_assert(bool) - single argument form | ||
| // CHECK-LABEL: @test_single_arg | ||
| // CHECK: %[[C:.*]] = p4hir.const | ||
| // CHECK: %[[R:.*]] = p4corelib.static_assert %[[C]] | ||
| p4hir.func @test_single_arg() -> !p4hir.bool { | ||
| %true = p4hir.const #true | ||
| %result = p4hir.call @static_assert_1(%true) : (!p4hir.bool) -> !p4hir.bool | ||
| p4hir.return %result : !p4hir.bool | ||
| } | ||
|
|
||
| // Test: static_assert(bool, string) - two argument form | ||
| // CHECK-LABEL: @test_with_message | ||
| // CHECK: %[[C:.*]] = p4hir.const | ||
| // CHECK: %[[R:.*]] = p4corelib.static_assert %[[C]] | ||
| p4hir.func @test_with_message() -> !p4hir.bool { | ||
| %true = p4hir.const #true | ||
| %msg = p4hir.const "compile-time check" : !string | ||
| %result = p4hir.call @static_assert_0(%true, %msg) : (!p4hir.bool, !string) -> !p4hir.bool | ||
| p4hir.return %result : !p4hir.bool | ||
| } | ||
|
|
||
| // Test: lowering works regardless of condition value | ||
| // CHECK-LABEL: @test_false_lowering | ||
| // CHECK: %[[C:.*]] = p4hir.const | ||
| // CHECK: %[[R:.*]] = p4corelib.static_assert %[[C]] | ||
| p4hir.func @test_false_lowering() -> !p4hir.bool { | ||
| %false = p4hir.const #false | ||
| %result = p4hir.call @static_assert_1(%false) : (!p4hir.bool) -> !p4hir.bool | ||
| p4hir.return %result : !p4hir.bool | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| // RUN: not p4mlir-opt %s --p4hir-evaluate-static-assert 2>&1 | FileCheck %s | ||
|
|
||
| // Test: static_assert(false) emits compile-time error | ||
|
|
||
| #false = #p4hir.bool<false> : !p4hir.bool | ||
|
|
||
| module { | ||
| // CHECK: error: static assertion failed | ||
| p4hir.func @test_false_error() -> !p4hir.bool { | ||
| %false = p4hir.const #false | ||
| %result = p4corelib.static_assert %false : !p4hir.bool -> !p4hir.bool | ||
| p4hir.return %result : !p4hir.bool | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| // RUN: p4mlir-opt %s --p4hir-evaluate-static-assert | FileCheck %s | ||
|
|
||
| // Test: static_assert(true) is removed by EvaluateStaticAssertPass | ||
|
|
||
| #true = #p4hir.bool<true> : !p4hir.bool | ||
|
|
||
| module { | ||
| // CHECK-LABEL: @test_true_removed | ||
| // CHECK-NOT: p4corelib.static_assert | ||
| // CHECK: p4hir.return | ||
| p4hir.func @test_true_removed() -> !p4hir.bool { | ||
| %true = p4hir.const #true | ||
| %result = p4corelib.static_assert %true : !p4hir.bool -> !p4hir.bool | ||
| p4hir.return %result : !p4hir.bool | ||
| } | ||
|
|
||
| // Test: non-constant condition is left unchanged | ||
| // CHECK-LABEL: @test_non_constant_unchanged | ||
| // CHECK: p4corelib.static_assert | ||
| p4hir.func @test_non_constant_unchanged(%arg0: !p4hir.bool) -> !p4hir.bool { | ||
| %result = p4corelib.static_assert %arg0 : !p4hir.bool -> !p4hir.bool | ||
| p4hir.return %result : !p4hir.bool | ||
| } | ||
| } | ||
|
|
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.
Once the folding is removed (see other comment), let's use something along the lines of: