[PATCH] D47587: [RISCV] Codegen support for atomic operations on RV32I

[Alex Bradbury via Phabricator via llvm-commits]

asb added a comment.

In https://reviews.llvm.org/D47587#1125426, @jyknight wrote:

> fence.tso is in the draft spec you linked to -- are you not using it because it hasn't been finalized, or was it just added after you left that comment?


Thanks for querying. I actually had been referring to an earlier draft. But even with the current draft I'm concerned that v2.0 of the ISA manual didn't specify that instr[31:28] should be ignored, meaning fence.tso may simply trap on some implementations. I'll query if anyone is tracking this behaviour across available RISC-V IP cores.



================
Comment at: test/CodeGen/RISCV/atomic-rmw.ll:495-496
+
+; Don't test for i8 atomicrmw {min,max,umin,umax} as Clang won't generate it
+; and we don't currently intend to support these with RV32A.
+
----------------
jyknight wrote:
> I think we ought to support these -- they're not much harder to implement than nand, I think. But I'll leave a comment on the other patch.
Not hard, just require another branch+BB and maybe an extra scratch register (there's no conditional move in RISC-V). So it does complicate the atomics lowering somewhat. But yes, it does feel like we should aim for completeness.


https://reviews.llvm.org/D47587