circt
circt copied to clipboard
[Arc] Add basic support for assert operation.
- Adds basic Assertion support to ARC
- Fixes: https://github.com/llvm/circt/issues/6810
We will implement this in two steps:
- [ ] Support the
assertoperation in Arc by addingarc.assert, and make sure it generates the correctLLVMcode. - [ ] Lower
verif.assertandsv.assert.concurrentoperations to Arc.
Aside from the conflicts, I will resolve them later:
Can you share your opinions?
how to generate the CHECK tests?
for basic-error.mlir : this is the error unexpected error: use of value '%in' expects different type than prior uses: 'i1' vs 'i4'
if I write expected-error @below {{use of value '%in' expects different type than prior uses: 'i1' vs 'i4'}} why doesn't it work?
for the print message before the abort, I want to make something like call i32 @puts(ptr @.str)
can you give some hints about converting the message to Value?
I tried LLVM::createPrintStrCall, but it didn't work here.
how to generate the
CHECKtests?
You typically write them by hand. You could, however, run the RUN command at the start of the file in the CLI, copy-paste it to the test file, and insert the CHECK prefixes, etc to save some typing. Don't forget to double check if the output actually matches what you expect.
for basic-error.mlir : this is the error
unexpected error: use of value '%in' expects different type than prior uses: 'i1' vs 'i4'if I writeexpected-error @below {{use of value '%in' expects different type than prior uses: 'i1' vs 'i4'}}why doesn't it work?
Did you check the location of the error and insert the expected-error directly at the line before?
for the print message before the abort, I want to make something like
call i32 @puts(ptr @.str)can you give some hints about converting the message toValue? I triedLLVM::createPrintStrCall, but it didn't work here.
Can you give more detail about why createPrintStrCall didn't work?
The rough procedure is: create a global constant for the string, insert the puts function declaration, insert a 'addressof' to get a pointer to the global, pass its result to a call to puts.
You could take a look at the lowering pattern for arc.sim.emit for an example (it uses printf instead though).
how to generate the CHECK tests?
There's a bit of an art to this. You're trying to balance:
- Exactness/terseness of the test. Try to keep the test simple and only check what the test needs to test not unrelated features or those tested elsewhere.
- Robustness of the test. Ideally, the test should only fail for patches which actually cause the test to fail, not due to name changes, SSA value changes, or later IR additions. This is not possible to guarantee always. That's fine.
- Readability of the test. Hopefully, someone reading the test (or your future self) will understand what it is trying to test. Striving for (1) helps here. Comments may be necessary to explain what is going on.
Generally, your tests look good!
Usually the only thing to avoid is exact copy-pasting of output. As Martin suggests, this can be a great starting point. Sometimes it is a useful exercise to intentionally write the test from scratch to make sure you aren't inadvertently relying on incorrect output.
I think we need to implement like llvm update_test_checks.py for circt-opt.
@fabianschuiki, @maerhart ping :bell: !!
This operation looks a lot like the one proposed in #6982 We should discuss with @fabianschuiki if the TableGen declaration should be moved to the Verif dialect, or if it makes sense to have two almost identical operations in both dialects.
@maerhart I don't know what's wrong with the checker! I run clang-format on my local repo and I don't get any errors!!
In the github actions summary you can download a patch it produced.
Here it is:
diff --git a/lib/Conversion/ArcToLLVM/LowerArcToLLVM.cpp b/lib/Conversion/ArcToLLVM/LowerArcToLLVM.cpp
index 120a140..7e06ed8 100644
--- a/lib/Conversion/ArcToLLVM/LowerArcToLLVM.cpp
+++ b/lib/Conversion/ArcToLLVM/LowerArcToLLVM.cpp
@@ -185,8 +185,7 @@ struct AssertOpLowering : public OpConversionPattern<arc::AssertOp> {
rewriter.getBoolAttr(true)));
rewriter.replaceOpWithNewOp<scf::IfOp>(
- op, negateCondition,
- [&](OpBuilder &builder, Location loc) {
+ op, negateCondition, [&](OpBuilder &builder, Location loc) {
// If the condition doesn't hold then print the message and abort
auto message = op.getMsg().value_or("");
auto module = op->getParentOfType<ModuleOp>();