riscv-non-isa/tg-nexus-trace

Feedback on RISC-V N-Trace Specification 1.0.0 rc2.0 - Chapter 2

Closed this issue · 2 comments

  1. Please update the terminology used in Chapter 2 (and the rest of the
    specification) as follows:

    itype Description
    0 No special type
    1 Exception
    2 Interrupt
    3 Trap return
    4 Not-taken branch
    5 Taken branch
    6 Indirect jump (with or without linkage)
    7 Reserved
    8 Indirect call
    9 Direct call
    10 Indirect jump (without linkage)
    11 Direct jump (without linkage)
    12 Co-routine swap
    13 Function return
    14 Other indirect jump (with linkage)
    15 Other direct jump (with linkage)

  2. Both ECALL and EBREAK cause synchronous exceptions, and are not considered
    to retire. The epc reported by both is the pc of the instruction itself and
    not the address of the following instruction. In table 2 - "ecall is
    reported after retirement" is wrong as ecall does not "retire".

  3. For 4-bit itype encodings, The rd != link should be classified further
    into two classes - 15 (direct jump with linkage) when rd != x0 and 11
    (direct jump without linkage) when rd = x0.

  4. c.j and cm.jt are misclassified. They should be classified as type 11
    (direct jump without linkage) and not 15.

  5. "If Pop returns the same address as PC at next valid ingress
    port cycle, emit Indirect Branch message with B-TYPE=0.". This needs to be
    corrected as the Indirect Branch messages hould be generated if the PC is
    not the same as the address Poped from the return address stack.

  6. Table 3 - "Add 0 as least significant bit to HIST field.". Consider updating
    to just "Update HIST field" since HIST is not introduced yet.

  7. "Encoder must handle call stack action as described in the Implicit Return
    Optimization chapter." Update to clarify that the encoder must handle call
    stack actions only if the Implicit return Optimization is implemented.

  8. The last non-normative note - "If optional trTeInstEnAllJumps..." should be
    a normative statement.

Editorial and typos:
9. "Table below provides.." -> "The table below provides.."

11."detailed mapping of of causes" -> remove repeated of. This paragraph can
also use rewriting for clarity. Use "operands" instead of "arguements".

  1. All instructions should use uppercase. For instance, "mret" -> "MRET".

13."N-trace is using the..." -> "N-trace uses the same..."

14."detailed mapping of of" -> "detailed mapping of"

15."When ingress port is implemented as 4-bit" -> "When the itype input of
the ingress port is 4-bit wide"

16.Table 4 - Reseved -> Reserved

17."other instruction" -> "Other instructions"

  1. "not listed in this table" - "All other instructions that are not listed in
    this table".

  2. "Implicit x1" -> The "Expands to jal x1, offset"

  3. "N-trace encoder does not require cause ..." to
    "The N-Trace encoder does not require 'cause' and 'tval' ingress port
    signals, which are valid only for exceptions and interrupts, as these
    details are not reported in N-Trace messages. Instead, N-Trace solely
    provides the address of the exception or interrupt handler."

  4. "As almost every ingress port..." to
    "Since almost every ingress port cycle updates I-CNT, there is a possibility
    of overflow. For more information, see the I-CNT Details chapter regarding
    I-CNT management and overflow handling."

  5. "If the optiona trTeInstEnAllJumps..." to
    "If the optional trTeInstEnAllJumps bit is set, the trace ingress port is
    required to report itype=5 (Taken branch) for all direct unconditional
    jumps, which are normally reported as itype = 0 or itype = 15". See also
    comment 8.

First point (unification of 'itype' is still TODO).

Closing this issue as these new names (with linkage) are now done (in some earlier commit).