AR: Define mcontrol* triggers and multiple accesses #883

timsifive · 2023-09-22T18:58:04Z

mcontrol: Behavior is undefined because it's deprecated.

mcontrol6: If a trigger reports hit=before then some memory accesses may have been performed. If it reports immediately after, then all accesses have already been performed.

gfavor · 2023-09-23T00:52:22Z

xml/hwbp_registers.xml

@@ -378,6 +378,9 @@
            though the load will not update its destination register. Debuggers
            should consider this when setting such breakpoints on, for example,
            memory-mapped I/O addresses.
+
+            It is undefined what should happen when the instruction in question


Saying that the behavior is "undefined" means that the possibilities are architecturally unbounded. Any behavior would be allowed - which is both undesirable and useless to some debugging code with such instructions.

Conversely, I would expect people that care about the Zc* instructions would (rightly) object to freeze (let alone ratification) of Debug 1.0 since it would be unusuable on embedded software that makes use of these instructions.

What is the right term here? Unspecified?

mcontrol triggers are already deprecated, but debuggers should know about them so they're in the spec. New hardware implementations should implement mcontrol6 instead, where the behavior is specified. Is that OK, or should I invent something for people insisting on mcontrol triggers with Zc*?

gfavor · 2023-09-23T00:54:41Z

xml/hwbp_registers.xml

+            before. This might mean that when the instruction is executed again
+            it will perform some of the same stores again (e.g. \tt{cbo.zero}),
+            or the instruction might have a mechanism that avoids duplicating the
+            stores (e.g. vector stores).


The "vector store" reference leaves the crucial piece of information a mystery to the reader unless they are intimately familiar with RVV. The parenthetical should say:

(e.g. vector stores and the vstart mechanism).

Done. Thanks.

pdonahue-ventana · 2023-09-25T20:52:06Z

xml/hwbp_registers.xml

+            It is \unspecified what should happen when the instruction in question
+            performs multiple memory accesses.


What should happen is only unspecified with respect to the memory accesses, not with respect to anything else.

Suggested change

It is \unspecified what should happen when the instruction in question

performs multiple memory accesses.

If an instruction matches this trigger and the instruction performs

multiple memory accesses, it is \unspecified which memory accesses

have completed before the trigger fires.

Much better. Thanks.

xml/hwbp_registers.xml

gfavor · 2023-09-26T00:45:34Z

xml/hwbp_registers.xml

+            before. This might mean that when the instruction is executed again
+            it will perform some of the same stores again (e.g. \tt{cbo.zero}),
+            or the instruction might have a mechanism that avoids duplicating the
+            stores (e.g. vector stores and the vstart mechanism).


This focuses on stores, but what about register results of an instruction that performs multiple memory accesses?

Must any and all register results be performed only if the instruction completes, i.e. before timing disallows any register modifications (that might, for example, prevent restartability of the instruction aupon return from the debug handler), and after timing of course requires all such modifications to have been performed.

And what about loads? Are some allowed, like stores, to have been performed under before timing? And what if some of those loads are to non-idempotent memory?

Lastly, cbo.zero is not the best of example since it architecturally performs a single block-size memory write. (An implementation can choose to implement that as a series of smaller stores of some size, but the expected common implementation would be one memory write.)

It seems like this paragraph needs to cover read acesses, write accesses, and register writes. Whether the statement is common to all three, or is a separate statement for register writes and for memory accesses, or maybe different for all three, all three need to be covered explicitly in some way so that peopel know what are the bounds on allowed and disallowed behavior for before triggers and for after triggers.

The first sentence is the important one. "If an instruction partially executed, the reported timing is still before." The rest is just example. I'll tweak the language to allow for more implementations. Do you have a suggestion for an instruction that performs multiple operations but that doesn't have a restart mechanism? I can't think of any.

There's no obvious perfect statement one coudl make. But one needs to inform users what is and isn't possible. Obviously users prefer a more useful statement that will allow most instructions to be restartable.

For example (and I'm not trying to say that this is what should be specified - the TG needs to consider the pros and cons of whatever it would choose to specify), one could say that some of the memory accesses (loads and stores) may have been performed but no registers will have been modified. And one should also note that while most instructions will still always be restartable, some may not be (and accesses to non-idempotent memory may be repeated).

I've rewritten almost all of the mcontrol6 before timing. Can you see if this is more clear?

gfavor · 2023-09-26T18:40:35Z

xml/hwbp_registers.xml

-            retired, but after all preceding instructions are retired.
+            The trigger fired before the instruction that matched it was
+            retired, but after all preceding instructions are retired. This
+            explicitly allows for instructions partially executing.


"for instructions to be partially executed."

gfavor · 2023-09-26T18:42:27Z

xml/hwbp_registers.xml

-            destination register.
+            An instruction that caused a trigger to fire might be executed
+            partially. In that case not all memory accesses may have been
+            performed, and some registers may not have been updated. Executing


Is this trying to say that ALL registers will not have been updated, or that some registers may have been updated?

I'm trying to say that anything partial is OK, from 0% to 100%. E.g. if we had an instruction that pops 3 registers from the stack, any of those 3 registers (or none of them) may contain the new value. The important thing for the debugger is that nothing breaks if we now resume off that instruction by letting it execute again. (Can't account for non-idempotent memory, however.)

@gfavor Is this OK, or would you like to see the language tweaked more?

mcontrol: Behavior is undefined because it's deprecated. mcontrol6: If a trigger reports hit=before then some memory accesses may have been performed. If it reports immediately after, then all accesses have already been performed.

pdonahue-ventana

Technically, nobody can guarantee that executing an instruction the second time is equivalent to executing it the first time because some other hart (or DMA or whatever) could have modified memory in between. But I think that the spirit is clear and it would just add unnecessary confusion to add any additional verbiage about other harts. So this is fine.

Incorporate more feedback from AR. Moved this into a separate Sdtrig section, since the discussion could also apply to instructions that cause triggers beside memory access ones. (E.g. an instruction that combines memory accesses with a trigger, and itrigger.) Follow up to #883.

gfavor · 2023-10-05T03:46:41Z

I assume Tim has now noticed that this PR probably wants to be reverted or replaced with a PR reflecting Oct 3rd AR feedback (email titled "Debug 1.0 AR next steps") about how best to more generally specify this issue.

timsifive · 2023-10-05T15:41:29Z

Yes. #899 is the update to this one.

timsifive requested a review from pdonahue-ventana September 22, 2023 18:58

timsifive added Clarification 1.0 labels Sep 22, 2023

timsifive force-pushed the multi_access branch from 0400361 to e1f7268 Compare September 22, 2023 19:07

gfavor reviewed Sep 23, 2023

View reviewed changes

timsifive force-pushed the multi_access branch from e1f7268 to f4cc02e Compare September 25, 2023 15:46

pdonahue-ventana reviewed Sep 25, 2023

View reviewed changes

gfavor reviewed Sep 26, 2023

View reviewed changes

timsifive force-pushed the multi_access branch 2 times, most recently from 3183a7e to c606bfb Compare September 26, 2023 18:26

gfavor reviewed Sep 26, 2023

View reviewed changes

AR: Define mcontrol* triggers and multiple accesses

443f05e

mcontrol: Behavior is undefined because it's deprecated. mcontrol6: If a trigger reports hit=before then some memory accesses may have been performed. If it reports immediately after, then all accesses have already been performed.

timsifive force-pushed the multi_access branch from c606bfb to 443f05e Compare September 26, 2023 18:52

pdonahue-ventana approved these changes Sep 28, 2023

View reviewed changes

timsifive merged commit 9ddcbcb into master Oct 4, 2023
1 check passed

timsifive deleted the multi_access branch October 4, 2023 16:29

timsifive mentioned this pull request Oct 4, 2023

AR: Clarify triggers and multiple state changes #899

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AR: Define mcontrol* triggers and multiple accesses #883

AR: Define mcontrol* triggers and multiple accesses #883

timsifive commented Sep 22, 2023

gfavor Sep 23, 2023

timsifive Sep 25, 2023

gfavor Sep 23, 2023

timsifive Sep 25, 2023

pdonahue-ventana Sep 25, 2023

timsifive Sep 26, 2023

gfavor Sep 26, 2023

timsifive Sep 26, 2023

gfavor Sep 26, 2023

timsifive Sep 26, 2023

gfavor Sep 26, 2023

timsifive Sep 26, 2023

gfavor Sep 26, 2023

timsifive Sep 26, 2023

timsifive Sep 28, 2023

pdonahue-ventana left a comment

gfavor commented Oct 5, 2023

timsifive commented Oct 5, 2023

		It is \unspecified what should happen when the instruction in question
		performs multiple memory accesses.

-            It is \unspecified what should happen when the instruction in question
-            performs multiple memory accesses.
+            If an instruction matches this trigger and the instruction performs
+            multiple memory accesses, it is \unspecified which memory accesses
+            have completed before the trigger fires.

AR: Define mcontrol* triggers and multiple accesses #883

AR: Define mcontrol* triggers and multiple accesses #883

Conversation

timsifive commented Sep 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pdonahue-ventana left a comment

Choose a reason for hiding this comment

gfavor commented Oct 5, 2023

timsifive commented Oct 5, 2023