asm: Add handling for atomic operations #1751
Conversation
acfcf12 to
c253f9c
Compare
|
So for signed division we extended the opcode space to be 16 bits. Does it make sense to use some of that space to encode LoadAcquire / StoreRelease as actual opcodes as well? I'm a bit torn because of that whole scheme you pointed out. We might end up with 2^5 additional opcodes in the worst case? I'm guessing that not all combinations are valid though. If we stick to the current approach, can we unexport AtomicFlags for now? That way we can punt on how they are encoded / whether we need further flags. |
Yes, I had forgotten we did that. I guess that makes more sense so we can avoid the special caseing in the string generation, which was the reason for doing this in the first place. I will give this a shot.
In theory. It seems that in practice there are 6 memory orderings commonly defined in C/CPP, so with a bit of luck we will never come close to 2^5 additional ops. What personally bothers me is that using odd schemes like offset and imm as part of the opcode is becoming more common. And while instructions are mostly still constant width, the encoding is moving away from the original and becoming less consistent. |
8e6b628 to
6cb882d
Compare
6cb882d to
81d04eb
Compare
e213316 to
4da3570
Compare
|
@dylandreimerink I pushed a commit which cuts down the repetition a bit. IIRC I added the free functions for Load, etc. because there wasn't that much variation. That isn't the case with the atomic ops so using a method on the type seems nicer? read okay to me, but I'm not 100% sure they are really correct. LoadAcquire and StoreRelease are also annoying. |
Ah, I think this is better. I had all of the different ops spelled out for the string generation. But that is something we can fix by checking for the
This works for the copy-modify-write style of atomics. We need to add a guard for The It technically works, its just that from a UX perspective they don't fit neatly in the
|
As it stands, we only had proper handling of atomic add operations. `lock *(u32 *)(r1 + 0x1) += w2` was the only instruction that was properly handled. Atomic operations have the same opcode, but and are differentiated by the imm value. So far we have not been looking at the imm value, so all atomic operations look like atomic adds to us. This commit adds decoding for all current atomic operations. We handle them similarly to how we handle ISAv4 instructions which use the offset to further specify the instruction. In #1193 we expanded the opcode from a u8 to u16, which is bigger than the actual size. This allowed us to still represent functionally different opcodes in Go, even tough the kernel uses other bits of the instruction. So our opcode in Go is not identical to the opcode in the kernel. We translate during marshalling and unmarshaling. So far, we have only needed a few additional bits, but the atomic ops need 9 bits of imm to fully encode all possibilities. Since 9 + 8 > 16 we have to grow the opcode to 32 bits. During unmarshaling, we simply take the lower 9 bits of the imm shift it left by 16 bits and or it with the opcode. During marshaling this process is reversed. Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
283ef55 to
8669dd4
Compare
Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
|
I am happy this this if you are. I say squash+merge 🚀 |
|
Thanks for iterating with me! |
As it stands, we only had proper handling of atomic add operations.
lock *(u32 *)(r1 + 0x1) += w2was the only instruction that was properly handled. Atomic operations have the same opcode, but and are differentiated by the imm value. So far we have not been looking at the imm value, so all atomic operations look like atomic adds to us.This commit adds decoding for all current atomic operations. We handle them similarly to how we handle ISAv4 instructions which use the offset to further specify the instruction.
In #1193 we expanded the opcode from a u8 to u16, which is bigger than the actual size. This allowed us to still represent functionally different opcodes in Go, even tough the kernel uses other bits of the instruction. So our opcode in Go is not identical to the opcode in the kernel. We translate during marshalling and unmarshaling.
So far, we have only needed a few additional bits, but the atomic ops need 9 bits of imm to fully encode all possibilities. Since 9 + 8 > 16 we have to grow the opcode to 32 bits.
During unmarshaling, we simply take the lower 9 bits of the imm shift it left by 16 bits and or it with the opcode. During marshaling this process is reversed.
Fixes: #1750