Skip to content

[WIP] [gen] Update baseline configuration for rmw#1578

Draft
ShaleXIONG wants to merge 22 commits intoherd:masterfrom
ShaleXIONG:chase-review
Draft

[WIP] [gen] Update baseline configuration for rmw#1578
ShaleXIONG wants to merge 22 commits intoherd:masterfrom
ShaleXIONG:chase-review

Conversation

@ShaleXIONG
Copy link
Copy Markdown
Collaborator

Add several missing rmw relaxations since such relaxation can match memory access such as M, R, and W.

this pull requisition contains #1569 too.

Comment thread gen/libdir/forbidden.conf Outdated
@ShaleXIONG ShaleXIONG force-pushed the chase-review branch 2 times, most recently from 3d3204a to e0f68d2 Compare November 19, 2025 10:57
Copy link
Copy Markdown

@chacon01 chacon01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ShaleXIONG! A couple of comments.

Comment thread gen/libdir/forbidden.conf Outdated
Comment thread gen/libdir/forbidden.conf Outdated
Comment thread gen/libdir/forbidden.conf Outdated
@ShaleXIONG
Copy link
Copy Markdown
Collaborator Author

ShaleXIONG commented Nov 20, 2025

There might a few things I might do before merge this pull, which might take a while:

  • introduce include syntax so we can separate the baseline and rmw-enhancement version.
  • Probably introduce syntax like [A|Q PosRW] which unfold to [A PosRW] and [Q PosRW]
  • Wait [gen] Introduce more wildcard syntax in diy #1516 for this to be merged so we can use more wildcard syntax which will help us simply all the syntax. It becomes increasingly difficult to review and auditing this file.

However, I am planning to leave this pull request open so you (@chacon01) can use it.

@ShaleXIONG ShaleXIONG force-pushed the chase-review branch 2 times, most recently from 56142b2 to 5243912 Compare February 18, 2026 17:43
Copy link
Copy Markdown

@chacon01 chacon01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amo and LxSx become tedious to enumerate the more the configuration file is restricted to prevent additional ordering. I see two issues:

  • Several cases where an existing effect could be an Amo or LxSx without changing the meaning
  • We can't create a general Rmw or Amo relaxation because it will include e.g. Amo.LdClr which has the value collision problem.

Perhaps the generator could support "user-defined" relaxations to help here e.g.

-define LdRmw LxSx|Amo.Swp|Amo.Cas|Amo.LdAdd
-define Rmw LdRmw|Amo.StAdd
...
-safe [A LdRmw PodW* ExpObs]
-safe [ExpObs PodWR Rmw L]

which would reduce the duplication everywhere else.

Additionally, perhaps an operation to make a relaxation "optional" e.g.

-safe [A Rmw? Pod** Rmw? ExpObs]

which would be equivalent to

-safe [A Pod** ExpObs]
-safe [A Rmw Pod** ExpObs]
-safe [A Pod** Rmw ExpObs]
-safe [A Rmw Pod** Rmw ExpObs]

Comment thread gen/libdir/forbidden.conf Outdated
Comment thread gen/libdir/forbidden.conf
Comment thread gen/libdir/forbidden.conf Outdated
# let ctrl = [Exp & R]; basic-dep; [BCC]; po
# let dob = ctrl; [Exp & W | HU | TLBI | DC.CVAU | IC]
-safe DpCtrldW
-safe [DpCtrldW ExpObs]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be aware that tying to ExpObs here will prevent it from combining with other relaxations that begin with ExpObs or Hat. We will need some way to allow ExpObs to combine with itself and with Hat e.g. [DpCtrldW ExpObs] + [ExpObs PodRW L] -> DpCtrldW Fre PodRWPL or [DpCtrlIsbdR ExpObs] + [Hat LxSx] -> DpCtrlIsbdR Hat LxSx. I think expressing the possible combinations in the configuration file would be tedious and error-prone, so it'll likely need to be some option for the generator.

Comment thread gen/libdir/forbidden.conf Outdated
Comment thread gen/libdir/forbidden.conf Outdated
Comment thread gen/libdir/forbidden.conf Outdated
Comment thread gen/libdir/forbidden.conf Outdated
Comment thread gen/libdir/forbidden.conf Outdated
-safe [A Amo.Cas L PodW* ExpObs] [A Amo.Cas L PosWR ExpObs]
-safe [A Amo.StAdd L PodW* ExpObs] [A Amo.StAdd L PosWR ExpObs]
-safe [A Amo.LdAdd L PodW* ExpObs] [A Amo.LdAdd L PosWR ExpObs]
-safe [A LxSx L PodW* ExpObs] [A LxSx L PosWR ExpObs]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[range([A];amo;[L])]; po; [Exp & M | Imp & Tag & R | MMU & FAULT] does not apply for LxSx, so this relaxation can be removed. While this will not produce allowed tests because we would need it to compose with a free Pod*R (that doesn't exist), it is inconsistent with the justifying comment.

For the Amo variants (and LxSx too), the generator will only compose these with the other relaxations which are properly ordered into the acquire read effect, so all the forbidden combinations will also be forbidden without the L.

Comment thread gen/libdir/forbidden.conf Outdated
Comment thread gen/libdir/forbidden.conf Outdated
@ShaleXIONG
Copy link
Copy Markdown
Collaborator Author

Amo and LxSx become tedious to enumerate the more the configuration file is restricted to prevent additional ordering. I see two issues:

  • Several cases where an existing effect could be an Amo or LxSx without changing the meaning
  • We can't create a general Rmw or Amo relaxation because it will include e.g. Amo.LdClr which has the value collision problem.

Perhaps the generator could support "user-defined" relaxations to help here e.g.

-define LdRmw LxSx|Amo.Swp|Amo.Cas|Amo.LdAdd
-define Rmw LdRmw|Amo.StAdd
...
-safe [A LdRmw PodW* ExpObs]
-safe [ExpObs PodWR Rmw L]

which would reduce the duplication everywhere else.

Additionally, perhaps an operation to make a relaxation "optional" e.g.

-safe [A Rmw? Pod** Rmw? ExpObs]

which would be equivalent to

-safe [A Pod** ExpObs]
-safe [A Rmw Pod** ExpObs]
-safe [A Pod** Rmw ExpObs]
-safe [A Rmw Pod** Rmw ExpObs]

User define is a good idea. I will probably use the let R = ... syntax in the hope I can copy-paste or reused some code in herd7.
The | syntax is introduced #1685 so it should be merged at some point.
Your proposal on ? is also a good idea. I will look into it.

@ShaleXIONG ShaleXIONG force-pushed the chase-review branch 2 times, most recently from 39a693a to 5c750cc Compare March 16, 2026 10:38
@ShaleXIONG ShaleXIONG force-pushed the chase-review branch 4 times, most recently from 90986fd to d133b19 Compare March 24, 2026 13:34
@ShaleXIONG ShaleXIONG force-pushed the chase-review branch 5 times, most recently from b1dc3c1 to 3dca6a9 Compare March 31, 2026 15:03
@ShaleXIONG ShaleXIONG force-pushed the chase-review branch 2 times, most recently from 3171945 to aa45f4d Compare April 9, 2026 08:44
Replace the old `LexUtil` split-based relax parser with a proper
lexer/parser pipeline that builds `Ast.t`. This adds support for:
- top-level and grouped choice with `|`. For example `PosWR|DpAddrdW`
  means either `PosWR` or `DpAddrdW`.
- optional relaxations with `?`
  For example `PosWR?`, means either `PosWR` or `` (empty).
- canonical AST normalization plus expansion to concrete sequences

Update `diy7`, `diycross7`, `diyone7`, `norm7`, `readRelax`, `logRelax`,
`edge`, and `relax` to consume the new AST-based parser, and remove the
old `lexUtil.mli` split API.
Add unit tests `gen/tests/test_parser.ml` and `.expected`.
- tests for three parsers
- tests for remove invalid relaxes.
ShaleXIONG and others added 15 commits April 27, 2026 10:11
…tests.

For `...;po` and `...;po;[M]` in cat, we append the external
communication edges `Obs` in the translation. Similarly for `po;...` and
`[M];po;...`, we prefix `Obs`.
Add back `Amo.StAdd` and `Amo.LdAdd` as the representation for all other
atomic operation. We also remove `[Q Amo.* ***]` as `Q` cannot apply to
`Amo.*`
Add more consecutive none fault check. This rule out undesired situations
where a memory access does not fault but the consecutive one faults.
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.

2 participants