xh1-research-fork/research/.xh1/runs/20260826T135336Z/review.md

The document claims to be an RTL-ready specification for the XH-1 memory subsystem and atomics, but it contains several significant technical errors, unsupported claims, and specification violations that undermine its credibility. The most critical issues are:

• Non‑standard FENCE encoding – FENCE Tso,Rw is not a valid RISC‑V fence instruction. The RVWMO model defines only I, O, R, W bits for predecessor/successor sets. Using a non‑standard field without marking it as a custom extension directly contradicts the stated RVWMO compliance.
• Incorrect FENCE semantics – FENCE R,Rw is described as providing release semantics for preceding writes. This is false; FENCE R,RW orders only earlier reads, not earlier writes. Release semantics for stores would require ordering earlier writes, i.e., FENCE W,RW or equivalent.
• Hardware backoff on SC failure – The document states that “SC failures trigger a backoff algorithm,” implying the hardware automatically retries the SC. The RISC‑V architecture leaves SC retry entirely to software; the hardware only returns a success/failure value. The described mechanism is not standard and would break the architectural contract.
• Unsupported performance numbers – The table in Section 5 presents precise latency, retry rate, and bandwidth figures without any methodology, simulation environment, workload, or citation. These are presented as factual measurements, not as projections or assumptions, and are therefore unsupported.
• Misleading ordering description – “Memory transactions are ordered by: …” lists arbitration priorities, not a global ordering guarantee. The phrasing is inconsistent with the later statement that the hardware does not enforce global program order, and it could mislead a reader into thinking the NoC provides a total order.
• Confusing MOESI terminology – Proposal 3 mentions promoting lines to a “shared‑dirty” state, but the standard MOESI protocol already defines the Owned state for dirty shared data. The document does not clarify whether this is a new custom state or a misnomer, and it bypasses the established protocol terminology.

These issues are substantive. The FENCE errors alone would cause a non‑compliant implementation, and the SC backoff claim reveals a fundamental misunderstanding of the ISA. The document is not ready for RTL implementation.

{
  "verdict": "FAIL",
  "confidence": "HIGH",
  "issues": [
    "FENCE Tso,Rw is not a standard RISC‑V fence encoding; the bits are I/O/R/W only. This violates RVWMO compliance unless explicitly marked as a custom extension.",
    "FENCE R,Rw is described as providing release semantics for preceding writes, but it only orders earlier reads, not earlier writes. The description is technically incorrect.",
    "The claim that SC failures trigger a hardware backoff algorithm contradicts the architecturally defined behaviour (SC returns 0/1, software retries). This is an ISA-level error.",
    "Performance numbers in Section 5 (AMO latency, retry rate, stall cycles, bandwidth) are presented as factual without any evidence, simulation setup, or citations.",
    "The text 'Memory transactions are ordered by: …' confuses arbitration priority with an ordering guarantee, which is inconsistent with the later statement that no global order is enforced.",
    "The 60% reduction in directory footprint from bitvector compression is asserted without supporting data or citation.",
    "Proposal 3 introduces a 'shared-dirty' state without reference to the existing MOESI Owned state, causing confusion about the coherence protocol."
  ],
  "required_fixes": [
    "Correct all FENCE encodings to use standard I/O/R/W bits, or explicitly define any custom fence variants as non‑standard extensions and mark them as such.",
    "Fix the description of FENCE R,Rw to accurately reflect that it orders only earlier reads, not earlier writes, and describe the corresponding acquire/release semantics correctly.",
    "Clarify that the backoff algorithm for SC retries is a software convention, not a hardware mechanism, and align the text with the RISC‑V LR/SC specification.",
    "Provide the simulation methodology, benchmarks, and configuration for the performance numbers, or clearly label them as preliminary estimates/projections.",
    "Replace 'Memory transactions are ordered by:' with a correct description of the NoC arbitration priority scheme, and remove the implication of a global ordering.",
    "Provide evidence for the claimed directory footprint reduction or mark it as an engineering estimate.",
    "Reconcile the 'shared-dirty' terminology with the standard MOESI states (e.g., Owned), or clearly define it as a new custom state with justification."
  ]
}