dialect: Fix riscv verify

[kingiler]

No description provided.

[math-fehr]

I'm fine for the irdl.py part, but I think you should split the changes, so we have a PR with the irdl.py changes, and one with the riscv changes, which are not that related?

[kingiler]

riscv are the proof that previous implementation has some bug. I'm adding them as a test case.

[compor]

riscv are the proof that previous implementation has some bug. I'm adding them as a test case.

It was not a bug per se, just an assert. Please split up the PRs.

[codecov]

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.04 🎉

Comparison is base (d8f78bc) 89.34% compared to head (f7387e3) 89.39%.

❗ Current head f7387e3 differs from pull request most recent head 41bb9cb. Consider uploading reports for the commit 41bb9cb to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1271      +/-   ##
==========================================
+ Coverage   89.34%   89.39%   +0.04%     
==========================================
  Files         181      181              
  Lines       24080    24084       +4     
  Branches     3663     3665       +2     
==========================================
+ Hits        21515    21530      +15     
+ Misses       1991     1980      -11     
  Partials      574      574              

Impacted Files	Coverage Δ
tests/dialects/test_riscv.py	100.00% <100.00%> (+7.53%)	⬆️
xdsl/dialects/riscv.py	83.83% <100.00%> (+0.07%)	⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[kingiler]

I think it is a bug. SomeOperation.verify() does not raise VerificationException but DiagnosticException. Because of this, the check_float_register is not named test_float_register in order bypass pytests in previous PR by others.

[kingiler]

Appears in this PR https://github.com/xdslproject/xdsl/pull/1188/files#diff-070c5a8a21ea33e601f29becbb1305e5aeea2c45f5256f2e2d88e3efcd04cc10R237 by @adutilleul

[math-fehr]

There are two bugs in my opinion.
One that we were returning a DiagnosticException, and the other that a test was not properly named, and thus was never tested (it could have just checked for DiagnosticException instead).

If we want to have a test for the irdl.py file, we should do so in test_irdl.py, and write a test for exactly that. Otherwise, the testing infrastructure of irdl.py depends on riscv tests, which might change in the future without a change in irdl.

So I would write a test in test_irdl.py, and have the riscv changes in another PR.

[kingiler]

Let me make separate PR.

[kingiler]

@math-fehr @compor I think it is ready for review.

Resolved