Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

i#6662 public traces: disable read/write counts in invariant_checker #7140

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

edeiana
Copy link
Contributor

@edeiana edeiana commented Dec 16, 2024

We previously relied on load and store categories of REGDEPS instructions
to indicate the presence of reads and writes that follow.
Unfortunately, not all instructions that then perform a read or a write have a
corresponding load or a store category, leading to false positive
"Too many reads/writes" invariants firing.

e.g., no store category for this instruction that performs a write operation:

13123537    10416044:        7446 ifetch       4 byte(s) @ 0x0000aaaaefe73500 00000831 0e0d0d04 load [4byte]       %rv11 %rv12 %rv13 -> %rv11
13123537    10416044:        7446                                             0000000f
13123538    10416044:        7446 read         4 byte(s) @ 0x0000115cffc33ba0 by PC 0x0000aaaaefe73500
13123539    10416044:        7446 write        4 byte(s) @ 0x0000115cffc33ba0 by PC 0x0000aaaaefe73500

We disable these invariants for REGDEPS traces.

Issue #6662

We previously relied on load and store categories of REGDEPS
instructions to indicate the presence of reads and writes that
follows.
Unfortunately, not all instructions that then perform a read or a
write have a corresponding load or a store category, leading to
false positive "Too many reads/writes" invariants firing.

e.g., no store category for this instruction that performs a write
operation:
```
13123537    10416044:        7446 ifetch       4 byte(s) @ 0x0000aaaaefe73500 00000831 0e0d0d04 load [4byte]       %rv11 %rv12 %rv13 -> %rv11
13123537    10416044:        7446                                             0000000f
13123538    10416044:        7446 read         4 byte(s) @ 0x0000115cffc33ba0 by PC 0x0000aaaaefe73500
13123539    10416044:        7446 write        4 byte(s) @ 0x0000115cffc33ba0 by PC 0x0000aaaaefe73500
```

We disable these invariants for REGDEPS traces.

Issue #6662
@@ -809,26 +809,10 @@ invariant_checker_t::parallel_shard_memref(void *shard_data, const memref_t &mem
instr_writes_memory(noalloc_instr);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, not all instructions that then perform a read or a write have a
corresponding load or a store category, leading to false positive
"Too many reads/writes" invariants firing.

Wait a minute: this does not sound right. Every instruction doing a load or store should have that category: that's the point of the category. I thought the problem was the other way around, instructions with variable number of loads/stores: e.g., masked or predicated scatter/gather.

e.g., no store category for this instruction that performs a write operation:

13123537 10416044: 7446 ifetch 4 byte(s) @ 0x0000aaaaefe73500 00000831 0e0d0d04 load [4byte] %rv11 %rv12 %rv13 -> %rv11
13123537 10416044: 7446 0000000f
13123538 10416044: 7446 read 4 byte(s) @ 0x0000115cffc33ba0 by PC 0x0000aaaaefe73500
13123539 10416044: 7446 write 4 byte(s) @ 0x0000115cffc33ba0 by PC 0x0000aaaaefe73500

This is a bug, isn't it? What's the original opcode? Sounds like a category bug in DR's decoder.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, it's a bug in the categories of some AARCH64 instructions that propagates into their corresponding REGDEPS instructions.
Bug filed: #7146.
Holding on this change until we know we can solve it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants