Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/test/CodeGen/LoongArch/ir-instruction/atomic-cmpxchg.ll | ||
---|---|---|
21 | Jiaxun told me (via linux-mips):
In GCC we use dbar 0x700, so in the future HW engineers can fix this issue and make dbar 0x700 no-op. |
llvm/test/CodeGen/LoongArch/ir-instruction/atomic-cmpxchg.ll | ||
---|---|---|
21 | Ouch, I mean the dbar 1792 instruction at LBB0_3. Yes this one can be removed for 3A5000. But what should we do if LLDBAR bit is 0 in CPUCFG? |
llvm/test/CodeGen/LoongArch/ir-instruction/atomic-cmpxchg.ll | ||
---|---|---|
21 | Nice question. What case we need memory barrier before atomic-op? Atomic-op with store-release semantics? Why not make sc as membar + store-conditional? If memory barrier semantics of ll are still membar + load-exclusive in future, and the atomic-op with load-acquire semantics, I think we can't make dbar 0x700 as no-op. |
LGTM. Maybe this is suboptimal to cover all current and future LoongArch models, but let's move on to finish basic functionalities and continue to polish the current implementation.
Let's wait a couple of days to see if others have more inputs.
In general this looks good, as we're still kinda experimental, it's probably better to land so we get wider test coverage, e.g. currently this is blocking bootstrap of LLVM/Clang.
Wait. I didn't see any processing wrt the return value of cmpxchg. Would you mind adding some test cases where the %res is not optimized out, so we can get to verify the return value behavior is in line with expectations?
llvm/test/CodeGen/LoongArch/ir-instruction/atomic-cmpxchg.ll | ||
---|---|---|
202 | Okay so my suspicion is confirmed, and the implementation is wrong... the [[ https://llvm.org/docs/LangRef.html#cmpxchg-instruction | result of a strong cmpxchg ]] should be 1 if $a3 == $a1, and 0 otherwise. It shouldn't be the original value. Please check if I'm misunderstanding anything... |
llvm/test/CodeGen/LoongArch/ir-instruction/atomic-cmpxchg.ll | ||
---|---|---|
202 |
The return values of cmpxchg include {original value, i1}, which is checked in this test case for original value. %res = extractvalue { i32, i1 } %tmp, 0 ret i32 %res |
llvm/test/CodeGen/LoongArch/ir-instruction/atomic-cmpxchg.ll | ||
---|---|---|
202 | Ah I see. It's compound. Then how about some more tests checking the i1 part too? |
llvm/test/CodeGen/LoongArch/ir-instruction/atomic-cmpxchg.ll | ||
---|---|---|
202 | Sure, I'll add test cases. |
llvm/test/CodeGen/LoongArch/ir-instruction/atomic-cmpxchg.ll | ||
---|---|---|
327 | This seems wrong: the following xor insn responsible for the same value -> 0 part operates on full GRLen, but here a1 i.e. %cmp gets zero-extended to a0, yet a3 i.e. the value from memory is loaded with ll.w that sign-extends. I expect this will fail for cases where cmp == *ptr && cmp < 0. Either this should be a sign-extension, or the comparison shouldn't be xor. |
llvm/test/CodeGen/LoongArch/ir-instruction/atomic-cmpxchg.ll | ||
---|---|---|
327 | Thanks for the fact checking! I will fix it. |
Not needed.
ll: full-membar + load-exclusive