Skip to content

Conversation

@jordancarlin
Copy link
Contributor

@jordancarlin jordancarlin commented Oct 27, 2025

Replace pseudoinstructions inline in fence.yaml with hint references.

Fixes #1181

@ThinkOpenly
Copy link
Collaborator

tsk, tsk. CI found trailing whitespace. :-)

@codecov
Copy link

codecov bot commented Oct 27, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 46.05%. Comparing base (3d9129d) to head (15c37c5).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1210   +/-   ##
=======================================
  Coverage   46.05%   46.05%           
=======================================
  Files          11       11           
  Lines        4942     4942           
  Branches     1345     1345           
=======================================
  Hits         2276     2276           
  Misses       2666     2666           
Flag Coverage Δ
idlc 46.05% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Replace `pseudoinstructions` inline in `fence.yaml` with hint references
@jordancarlin
Copy link
Contributor Author

tsk, tsk. CI found trailing whitespace. :-)

Oops. Fixed. Seems like the ISS is failing now. I'll look into it later.

@dhower-qc
Copy link
Collaborator

@henrikg-qc, ISS is getting an undefined value error. It's not obvious to me from the diff why.

@jordancarlin
Copy link
Contributor Author

I was looking into this more locally, and so far I haven't found anything useful. The backtrace doesn't go back past the ruby, so it's hard to track down what is going wrong.

@ThinkOpenly
Copy link
Collaborator

The problem appears to stem from the unknown value of menvcfg.FIOM field, which the ISS conveniently detects. In this particular case, however, those bits are not relevant to the failing test.

Suggested resolution is to create a new parameter for the reset value of FIOM, and set that parameter in the config file for riscv-tests. Jordan said he'd try this when he got time.

@jordancarlin
Copy link
Contributor Author

@dhower-qc @henrikg-qc @ThinkOpenly I started looking into adding a parameter for the reset value of menvcfg.FIOM like we discussed last week, and discovered another issue. The menvcfg CSR is only defined for Sm >= 1.12.0, but the rv32-riscv-tests config specifies that it implements Sm 1.11.0. That means that it isn't possible to define the parameter for that config, but more importantly, it means that the menvcfg CSR shouldn't be accessible at all when running these tests.

I'm thinking about adding a check in the fence IDL implementation so that it only attempts to access the FIOM bit if Sm >= 1.12.0 is implemented, but this seems likely to be a wider issue so I'm wondering if there is a better global fix for this kind of thing.

@henrikg-qc
Copy link
Collaborator

There is a global fix for this pending. The ISS built for the regression tests will run with compiler definition IGNOREDEFAULTS. This will not throw the UndefinedValueError exception.
Everything is implemented in the build flow but I have a rake issue I am working through. I will push this for review shortly.

@ThinkOpenly
Copy link
Collaborator

The menvcfg CSR is only defined for Sm >= 1.12.0, but the rv32-riscv-tests config specifies that it implements Sm 1.11.0.

That's a bit unexpected, to me at least. Do we know why that restriction exists?

@jordancarlin
Copy link
Contributor Author

The menvcfg CSR is only defined for Sm >= 1.12.0, but the rv32-riscv-tests config specifies that it implements Sm 1.11.0.

That's a bit unexpected, to me at least. Do we know why that restriction exists?

From the "Preface to Version 20211203" documenting the changes since priv 1.11:

Defined menvcfg, henvcfg, and senvcfg CSRs (and RV32-only menvcfgh and henvcfgh CSRs), which control various characteristics of the execution environment.

I assume it just wasn't needed by any features before priv 1.12 was introduced.

@ThinkOpenly
Copy link
Collaborator

The menvcfg CSR is only defined for Sm >= 1.12.0, but the rv32-riscv-tests config specifies that it implements Sm 1.11.0.

That's a bit unexpected, to me at least. Do we know why that restriction exists?

From the "Preface to Version 20211203" documenting the changes since priv 1.11:

Defined menvcfg, henvcfg, and senvcfg CSRs (and RV32-only menvcfgh and henvcfgh CSRs), which control various characteristics of the execution environment.

I assume it just wasn't needed by any features before priv 1.12 was introduced.

I didn't state my concern clearly... Why does riscv-tests need to be restricted to Sm 1.11.0? Can it just use Sm 1.12.0?

@jordancarlin
Copy link
Contributor Author

I didn't state my concern clearly... Why does riscv-tests need to be restricted to Sm 1.11.0? Can it just use Sm 1.12.0?

Good question. I’m not sure why that config is set to 1.11. It probably makes the most sense to set Sm to 1.13.0 (the latest version). @henrikg-qc any particular reason it was set to 1.11?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add pause and prefetch instructions

4 participants