Short-sighted size definition for atomics
jrtc27 opened this issue · 4 comments
It's true that W and D only exist currently. However having the format not be more flexible as with normal loads and stores seems short-sighted; for example, B and H atomics have value and may well be added as an extension (and our custom extension adds them because we fundamentally need them). Similarly 128-bit atomics may exist in future. I suggest just using data_width_p as with loads and stores; this also likely allows for simpler parsers (and implementations emitting the trace) as the logic for loads and stores can be reused.
Jessica,
Sorry, but this email feels like it is the response to an ongoing discussion or something, and I’m not aware of having seen anything earlier (apologies if I’ve missed something).
There isn't, I just took a look at this spec after it was mentioned in yesterday's chairs meeting.
Can you provide more context and detail please? What are ‘W’ and ‘D’ for example?
By W and D I mean 32-bit and 64-bit atomics (AMOADD.W/D etc). The current trace spec allocates a single bit for atomic instructions to indicate what size of data they are operating on, and my concern is not allowing for the same flexibility as normal loads/stores with data_width_p (and its log2) will become a problem; it would definitely be a barrier to us adopting this spec for our custom extension, where we have (and fundamentally need for correctness reasons) 8-bit and 16-bit LR/SC instructions (we don't add actual AMOs, simply because we want to get performance as comparable to plain RISC-V as possible), as well as sort-of 128-bit atomics (without going into the details of our extension I can't really explain why it's "sort-of"), but there are also cases where RISC-V could benefit from a standard extension that adds 8-bit and 16-bit atomics so it might in future become a concern for RISC-V itself, not just us with our custom extension.
Fixed