Add lowering support for static_assert#326
Add lowering support for static_assert#326devalgupta404 wants to merge 2 commits intop4lang:mainfrom
Conversation
|
|
||
| // Ensure the condition is a compile-time constant | ||
| Attribute constAttr; | ||
| if (!matchPattern(args[0], m_Constant(&constAttr))) |
There was a problem hiding this comment.
This should be done differently. Per spec:
these functions require compile-time known values as arguments, thus they can be used to enforce compile-time invariants
So, the condition could really be resolved later and error out here is a premature.
We'd need to low:
- Lower the generic extern call into a (new) op with clear semantics
- Provide evaluation step (we can just constfold out things with positive assertions, but errors must be done in a separate pass to keep context)
Signed-off-by: devalgupta404 <devalgupta4@gmail.com>
Signed-off-by: devalgupta404 <devalgupta4@gmail.com>
devalgupta404
force-pushed
the
issue-183
branch
from
a263dec to
ae3988e
Compare
|
Hi @asl, |
|
@mtsamis can you review it please |
| return success(); | ||
| } | ||
| // Handle static_assert: lower to P4CoreLib::StaticAssertOp | ||
| if (callee.getLeafReference().getValue().starts_with("static_assert")) { |
There was a problem hiding this comment.
This is an insufficient check (e.g. there could be a user named function static_assert_condition that would pass this). static assert will be only present in an overload set that looks like this:
p4hir.overload_set @static_assert {
p4hir.func @static_assert_0(!p4hir.bool {p4hir.dir = #undir, p4hir.param_name = "check"}, !string {p4hir.dir = #undir, p4hir.param_name = "message"}) -> !p4hir.bool
p4hir.func @static_assert_1(!p4hir.bool {p4hir.dir = #undir, p4hir.param_name = "check"}) -> !p4hir.bool
}
So you should get the overload_set and then check if it's static_assert.
| if (!message) { | ||
| op.emitWarning("static_assert message is not a compile-time constant; ignoring"); | ||
| } | ||
|
|
There was a problem hiding this comment.
Remove empty line, plus formatting above is off, have you run clang-format?
Also, as the llvm code styling guideline suggests, please don't use brackets for simple one-line if statements.
| if (auto constOp = args[1].getDefiningOp<P4HIR::ConstOp>()) { | ||
| message = llvm::dyn_cast<StringAttr>(constOp.getValue()); | ||
| } | ||
| if (!message) { |
There was a problem hiding this comment.
Is this allowed to happen? (i.e. a non constant string argument).
If not, use assert/llvm_unreachable
| setNameFn(getResult(), "lookahead"); | ||
| } | ||
|
|
||
| OpFoldResult P4CoreLib::StaticAssertOp::fold(FoldAdaptor adaptor) { |
There was a problem hiding this comment.
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?
| if (!constOp) | ||
| continue; | ||
|
|
||
| auto boolAttr = llvm::dyn_cast<P4HIR::BoolAttr>(constOp.getValue()); |
There was a problem hiding this comment.
Since cond is BooleanType, let's use llvm::cast here and remove if (!boolAttr) continue;
| }); | ||
|
|
||
| for (auto op : assertOps) { | ||
| Value cond = op.getCond(); |
There was a problem hiding this comment.
No need for single use variable here, just do op.getCond().getDefiningOp... below
|
|
||
| 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} |
There was a problem hiding this comment.
Following the advice at the top, these must be in a overload_set, as it would be generated by translating the core.p4 definitions.
| evaluate to a constant boolean. If the condition is true, this op | ||
| can be folded away. If false, a separate pass will emit an error. | ||
|
|
||
| Optionally includes a message string for error reporting. |
There was a problem hiding this comment.
Once the folding is removed (see other comment), let's use something along the lines of:
Represents a compile-time assertion with an optional error message.
The condition must eventually evaluate to a constant boolean.
An error will be emitted if the condition evaluates to false.
3 participants
issue #183
Add lowering support for static_assert in the P4HIRToCoreLib conversion.
First I evaluates the assertion condition at compile time. If the condition is a constant boolean true, the call is replaced with p4hir.const #true. If the condition evaluates to false, a diagnostic (static assertion failed) is emitted during lowering.
Covers Four Test cases
I am using '--split-input-file' to run each cases in an independent module otherwise repeated definations of 'static_assert' will cause duplicate symbol errors.