On Fri, 14 Oct 2022 13:39:33 PDT (-0700), jeffreya...@gmail.com wrote:
On 10/14/22 05:03, Christoph Müllner wrote:
My guess is people like the ISA mapping (more) because it has been
documented and reviewed.
And it is the product of a working group that worked out the
RVWMO specification.
This gives some confidence that we don't need to rework it massively
because of correctness issues in the future.
This stuff can be hard and if someone with deep experience in memory
models has reviewed the ISA mapping, then I'd prefer it over the GCC
mapping. It's just more likely the experts in the memory model space
are more likely to get it right than a bunch of compiler junkies, no
matter how smart we think we are :-)
That's not really proven the case for the ISA suggested Linux mappings.
We've been through a bunch of rounds of review upstream and that's
resulted in some differences. Some of that is likely related to the ISA
mappings for Linux being incomplete (they punt on the tricky bits like
interactions with spinlocks, which filter all over the place), and Linux
doesn't have the same old binary compatibility issues (aka the in-kernel
ABI is not stable between releases) so mapping churn isn't nearly as
scary over there.
Still not much of an argument in favor of the GCC mappings, though. I'm
pretty sure nobody with a formal memory model backgound has looked at
those, which is pretty much the worst spot to be in. That said, I don't
think we can just say the ISA mappings are the way to go, they seem to
suffer from some similar incompleteness issues (for example, there's no
explicit mappings for std::atomic<T>::compare_exchange_{weak,strong}).
So we'll still need to put in the work to make sure whatever mappings
get implemented are correct.
[
As an aside, I think LLVM is doing the wrong thing for some of the more
complex forms of compare_exchange_weak. For example
#include <atomic>
bool f(std::atomic<long>& p, long& o, long n) {
return p.compare_exchange_weak(o, n, std::memory_order_acq_rel,
std::memory_order_release);
}
$ clang-15.0.0 -std=c++17 -O3
f(std::atomic<long>&, long&, long): # @f(std::atomic<long>&,
long&, long)
ld a4, 0(a1)
.LBB0_3: # =>This Inner Loop Header: Depth=1
lr.d.aq a3, (a0)
bne a3, a4, .LBB0_5
sc.d.rl a5, a2, (a0)
bnez a5, .LBB0_3
.LBB0_5:
xor a0, a3, a4
seqz a0, a0
beq a3, a4, .LBB0_2
sd a3, 0(a1)
.LBB0_2:
ret
doesn't look to me like it provides release ordering on failure, but I'm
not really a memory model person so maybe I'm missing something here.
The GCC mapping is pretty ugly, but I think we do happen to have correct
behavior in this case:
# gcc-12.2.0 -std=c++17 -O3
f(std::atomic<long>&, long&, long):
ld a5,0(a1)
fence iorw,ow; 1: lr.d.aq a4,0(a0); bne a4,a5,1f; sc.d.aq
a3,a2,0(a0); bnez a3,1b; 1:
sub a5,a4,a5
seqz a0,a5
beq a5,zero,.L2
sd a4,0(a1)
.L2:
andi a0,a0,1
ret
]
Maybe I'm being too optimistic, but it's not hard for me to see a path
where GCC and LLVM both implement the ISA mapping by default. Anything
else is just a path of long term pain.
I'd bet that most people want that, but in practice building any real
systems in RISC-V land requires some degree of implementation-defined
behavior as the specifications don't cover everything (even ignoring the
whole PDF vs specification word games). That's not to say we should
just ignore what's written down, just that there's more work to do even
if we ignore compatibility with old binaries.
I think the question here is whether it's worth putting in the extra
work to provide a path for systems with old binaries to gradually
upgrade to the ISA mappings, or if we just toss out those old binaries.
I think we really need to see how bunch of a headache that compatibility
mode is going to be, and the only way to do that is put in the time to
analyze the GCC mappings.
That said, I don't really personally care that much about old binaries.
Really my only argument here is that we broke binary compatibility once
(right before we upstreamed the port), that made a handful of people
mad, and I told them we'd never do it again. I think we were all
operating under the assumption that RISC-V would move an order of
magnitude faster that it has, though, so maybe we're in a spot where
nobody actually cares about extant binaries and we can just throw them
all out.
If we're going to do that, though, there's a bunch of other cruft that
we should probably toss along with the GCC mappings...
Jeff
