[herd] Use "static" po indices when rendering execution graphs#1628
[herd] Use "static" po indices when rendering execution graphs#1628fsestini merged 2 commits intoherd:masterfrom
Conversation
HadrienRenaud
left a comment
There was a problem hiding this comment.
I can't really speak about functionality, especially interaction with instruction fetching, but I have 3 little code comments
|
Thank you, @fsestini, this is an interesting idea. A few quick thoughts:
|
|
Thank you for looking into this @artkhyzha . Your first two questions in particular touch on something I thought about when working on this. I'll address all questions in a moment, but first let me add context on the reasons why I decided to implement this change at the time I did, which might help us understand where my proposal is coming from, and perhaps figure out alternative approaches. While I think it's useful in its own right to provide, in the execution graphs, a more reliable pointer into the instruction's location in the source litmus test code, the main motivation for this change is to support my work on Consider this litmus test: The expected output from To produce such description, we need:
Internally, The difficulty is with point (2), since currenly the instruction-related information
I have doubts this information alone is sufficient to reliably trace an Effect back to its source instruction in the litmus code. I suppose we could, in principle, find a way for The string representation of instructions as shown in the execution graphs is also not always fully usable by Among all the options that I could think of and that would allow a tool like Now, to your points:
As for I appreciate your question was posed in the context of herd7 alone, and in that context, I don't see particular problems with deriving static pois from addresses or vice versa (pre-#1518). From the point of view of A alternative approach could be to move the "static poi <-> address" translation logic out of
I suppose it would depend on how this sequence is structured, and what kind of information we can derive from it. I would be happy to consider adding different kinds of data to the execution graphs, if that data can be used to reliably trace effects back to their source instructions.
If this PR ends up reaching consensus and approval for merge well before #1518 does, we indeed might. Otherwise, I wouldn't mind holding back until after #1518 is merged and then rebasing this on top. This PR is quite small, and not super urgent as its mainly in support of
Agreed. Currently static pois are calculated by incrementing a counter for each
Could you clarify what you mean by "code" here? Would that be what |
This PR make some small changes to the way
herd7renders its dot execution graphs, in particular relating to thepoiindices.Motivation
Consider this litmus test:
If we run this test through with
herd7 test.litmus -o . -show all, we get a dot file that shows these events for one of the executions of P1:You can see that the poi index of STR W2,[X3] is 5, whereas if we look at the program purely syntactically, that instruction actually appears at index 4 (counting from 0), or actually index 3 if we ignore the label:
Indeed we can see that one of the other executions shown in the dot file does assign a poi = 3 to that same instruction:
My understanding of the rationale behind this is that in the presence of jumps like CBNZ, herd will assign addresses and poi indices (among other things) by considering both branch possibilities in sequence. Therefore, in this example, poi will be assigned as follows:
In other words, the
poihere is closer to a runtime program counter rather than a static index pointing to the instruction's position in the source code.There's clearly very good reasons for
herd7to use this "runtime" poi internally. However, from the point of view of a person looking at the execution graph, and comparing it to the source litmus test, I think it would be more useful if they were presented with apoindex that is "static", i.e. that corresponds to the position of instructions in the source code.Note that this is in fact what
herd7already prints in the graph when-showevents all -withbox trueare passed as options:You can see that the name of the subgraph, namely
cluster_proc1_poi3, shows the instruction's "static"poi(3), vs the actual node label showing the "runtime"poi(5).This PR slightly modifies the logic that builds
herd7's internal representation of the litmus test to analyse, so that information about each instruction's "static"poican be propagated to the code responsible for generating the execution graphs.For example, with this PR, the
subgraphshown above would look like so:NOTE: this PR changes how graphs are generated, for certain litmus tests. However, running
make teston my machine did not show any failing tests, which suggests that either I'm using the wrong command, or this PR does not impact functionality that the current test suite covers. That's not anything remotely alarming per se, as testing this functionality may just not be worth it. However, we might want to take the opportunity to ask ourselves that question.