Feedback on chapter 2 - RISC-V N-Trace (Nexus-based Trace) Specification
Closed this issue · 1 comments
This issue summarizes the feedback and comments from AR on Chapter 2.
-
There may be many purposes for a specification. The second part of the
below sentence should be removed:
"Table below provides a detailed mapping of encodings of instructions
into itype signal - it should be used during development of ingress
port logic inside of a hart."
Further, it is not right to say that the itype is an encoding of the
instructions. While some instructions may lead to a control tranfer
out of a block of instructions, other causes of transfer out of a
block of instructions exists.
Suggest to use "Table below provides a mapping of causes for
terminating an instruction block to the corresponding itype encoding" -
The title of the first column of table 2 is misleading by stating it
relates to the instruction that retired. As exceptions and traps are
included, the more appropriate heading would be "Cause for instruction
block termination". -
The term "Interrupted instruction" may be misleading. While
instructions may be interrupted in some cases such as with vector
instructions, interrupts usually interrupt the instruction stream and
occur between instructions. The distinction is not material for N-trace
and so stating "Interrupt" as the what caused transfer out of the block
suffices. Reword "Interrupted instruction" to "An interrupt trap occurred
following the final retired instruction in the block." -
The term "Exception in instruction" is misleading. While exceptions can
occur due to instruction execution, they may also occur during the act of
fetching an instruction - e.g., an instruction page-fault, an instruction
access fault, etc. Reword to "An exception trap that occurred following the
final retired instruction in the block". -
uret is not a ratified instruction and should be removed.
-
cm.jt is a indirect table jump. It should be classified as uninferrable jump
-
Since non-control-transfer instructions do not lead to a termination of the
block, why do we need to add a row for "non-jump". Is the expectation that
every instruction when N-trace is active produces a trace packet? -
cm.jalt is classified as a inferable call. However, it is unclear if the JVT
is always part of the instruction image. While cm.jalt treats accesses to
the JVT as implicit access the Zc allows for this to be dynamically located.
Should this be classiffied as a uninferrable call instead? -
Table 2-2 - first instance of cm.popret* - capitalizes Z in Zcmp
-
Readability of Chapter 2 would be greatly enhanced if the terms I-CNT
BTM, and HTM and their concepts were introduced early. -
Table 3 - I-CNT should not be updated by exception.
-
Table 3 - I-CNt should not be updated by interrupt. When the interrupt is in
boundary of two instructions, the previous instruction has updated the I-CNT
on its retirement. When an instruction itself is interruptible, then the I-CNT
should not be updated. -
Table 4 - The header row is missing?
-
Table 4 - what does Push and Pop signify?
-
Table 4 - What is meant by "pop returns the same address as PC"
-
Table 4 - statements like "at next ingress port cycle" imply assumption
about a certain implementation. The architecture should be describe based
on the ISA property and not an implementation. For example, would the
implementation be non-compliant if it could do many taken branches in a cycle?
Or if it sends the address after 2 cycles? -
The note about trTeInstEnaAllJumps implies that direct unconditional jumps
are reported as itype=0. Per Table 2 inferrable jumps use itype=15/14. Only
when implicit return optimization is not available they use itype=0 - should
this text be qualified by itype_width_p -
The specification suggests using a reserved encoding. Specifications should
not encourage use of reserved encodings to preserve them for future standard
use. A better recommendation might have been to use encoding of 5 instead of
0. That could avoid the - encode as 7 and interpret as 0 or 5 guideline. -
The cause and tval inputs being disabled should be stated normatively and
not in a informative note as it is an important aspect of N-trace. tval is
spelt incorrectly astvar. -
un-inferable -> uninferable
-
Table 2 - This table duplicates the information in the E-trace spec (table 4.4 and
section 4.1.1). There may be improvements needed in that table (e.g., updates for
Zc), but those should be made in the form of PRs to the E-trace spec. -
Page 9 - informative note - Only the comment about 4 bit itypes and implicit
return optimization is needed. Rest of this comment can be removed. -
Table 3 - Exception - Directive to emit 2 or 1 for BTYPE should be
qualified by what guides that decision. -
Table 3 - Exception - Clarify what "address emitted is known at next ingress
port cycle" -
Table 4 - Uninferable tail call - This is just an indirect jump (JALR) that
is not a call or return. Why is this not possible? This should match
behavior for itype=14. Same comment for itype=11/15
I am closing all N-Trace PDF related issues with same comments as all issues were handled via comments/discussions in SINGLE Google Docs. Relevant links are as follows:
Notes to N-Trace PDF: https://docs.google.com/document/d/1h__c0Kc7TQAWMh5bw9cNC9bl_IGqyY_ylPV14uc2xj0