Skip to content

Validation/parsing clarification: orderings on non-atomic ops #26

Open

Description

@alexcrichton

Currently the explainer of this proposal says:

It is a validation error if there is an ordering immediate present for any non-atomic instruction that uses a memarg (such as standard loads and stores).

And currently there is one test for this being an error but that's using assert_malformed (as opposed to assert_invalid) and it's also for the text format.

This led me to a question about this: should the overview change the phrase "validation error" to "parse error"? For example is it expected that parsers parse orderings on all memargs and the validator rejects them (validation error), or is it expected that parsers reject bit 4 being set for any non-atomic memargs (parse error). I'd personally lean towards the latter if I got to choose.

As a subsequent, but possibly related, follow-up: could some more spec tests be added about invalid binary forms? For example could assert_malformed (assuming my question above is resolved as "parse error") be added for the binary format? Additionally could assert_malformed be added for specifying 0x11 as an ordering for non-rmw ops, and then orderings like 0x10 and 0x01 are invalid for rmw ops? (assuming I'm understanding the explainer right). I'm assuming assert_malformed for these sorts of errors, but assert_invalid may also be more appropriate (I'm not sure myself, but I'd lean towards assert_malformed)

Activity

  1. rmahdav commented on Sep 23, 2026

    @rmahdav
    Collaborator

    Good catch! There is an inconsistency between the explainer stating this as a 'validation error' and the tests treating it as a 'parser error'.

    Looking at other tests and proposals, I agree with you that this looks more like a 'parser error'. Orderings on non-atomic instructions should not be decoded in the first place. So changing the explainer makes sense to me.

    What does everybody else think about making this change? @tlively @conrad-watt

  2. stevenfontanella commented on Sep 23, 2026

    @stevenfontanella
    Member

    Agreed that this should be a parse error.

    Re:

    could some more spec tests be added about invalid binary forms? For example could assert_malformed (assuming my question above is resolved as "parse error") be added for the binary format? Additionally could assert_malformed be added for specifying 0x11 as an ordering for non-rmw ops, and then orderings like 0x10 and 0x01 are invalid for rmw ops? (assuming I'm understanding the explainer right)

    Your assumptions are correct, and I agree. I'll add tests for these.

  3. tlively commented on Sep 24, 2026

    @tlively
    Member

    Making this a parse error (i.e. assert_malformed) sounds right to me, too.

  4. self-assigned this
    on Sep 24, 2026
  5. rmahdav commented on Oct 8, 2026

    @rmahdav
    Collaborator

    I updated Overview.md in #29 to clarify that setting bit 4 on non-atomic instructions is a decode/parse error rather than a validation error. Leaving this open so @stevenfontanella can add the remaining assert_malformed binary tests.

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

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions