Re: [PATCH v2 00/10] [RISC-V] Atomics improvements [PR100265/PR100266]

[Palmer Dabbelt <palmer@dabbelt.com>]

On Thu, 13 Oct 2022 15:39:39 PDT (-0700), gcc-patches@gcc.gnu.org wrote:
>
> On 10/11/22 17:31, Vineet Gupta wrote:
>>
>>>
>>> I expect that the pressure for a proper fix upstream (instead of a
>>> backward compatible compromise) will increase over time (once people
>>> start building big iron based on RISC-V and start hunting performance
>>> bottlenecks in multithreaded workloads to be competitive).
>>> What could be done to get some relief is to enable the new atomics
>>> ABI by a command line switch and promote its use. And at one point in
>>> the future (if there are enough fixes to justify a break) the new ABI
>>> can be enabled by default with a new flag to enable the old ABI.
>>
>> Indeed we are stuck with inefficiencies with status quo. The new abi
>> option sounds like a reasonable plan going fwd.
>>
>> Also my understand is that while the considerations are ABI centric,
>> the option to faciliate this need not be tied to canonical -mabi=lp32,
>> lp64d etc. It might just be a toggle as -matomic=legacy,2019 etc (this
>> is not suggestive just indicative). Otherwise there's another level of
>> blowup in multilib testing etc.
>
> If I understand the history here, we're essentially catering to code
> that is potentially relying on behavior that was never really
> guaranteed.   That's not really ABI -- it's more depending on specifics
> of an implementation or undefined/underdefined behavior.    Holding back
> progress for that case seems short-sighted, particularly given how early
> I hope we are in the RISC-V journey.
>
>
> But I'm also sympathetic to the desire not to break existing code. 

I suppose ABI means a ton of things, but in this case that's really want 
I was trying to get at: just changing to the mappings suggested in the 
ISA manual risks producing binaries that don't work when mixed with old 
binaries.  My rationale for calling that ABI was that there's a defacto 
memory model ABI defined as "anything that works with the old binaries", 
but ABI means so many things maybe we just shouldn't say it at all here?

I'm going to call it "old binary compatibility" here, rather than "ABI 
compatibility", just so it's a different term.

> Could we keep the old behavior under a flag and fix the default behavior
> here, presumably with a bit in the ELF header indicating code that wants
> the old behavior?

The thread got forked a bit, but that's essentially what I was trying to 
suggest.  I talked with Andrea a bit and here's how I'd describe it:

We have a mapping from the C{,++}11 memory model to RVWMO that's 
currently emitted by GCC, and there are years of binaries with that 
mapping.  As far as we know that mapping is correct, but I don't think 
anyone's gone through and formally analyzed it.  Let's call those the 
"GCC mapping".

There's also a mapping listed in the ISA manual.  That's not the same as 
the GCC mapping, so let's call it the "ISA mapping".  We need to 
double-check the specifics, but IIUC this ISA mapping is broadly 
followed by LLVM.  It's also very likely to be correct, as it's been 
analyzed by lots of formal memory model people as part of the RVWMO 
standardization process.

As far as I know, everyone likes the ISA mapping better than the GCC 
mapping.  It's hard to describe why concretely because there's no 
hardware that implements RVWMO sufficiently aggressively that we can 
talk performance, but at least matching what's in the ISA manual is the 
way to go.  Also, just kind of a personal opinion, the GCC mapping does 
some weird ugly stuff.

So the question is really: how do we get rid of the GCC mapping while 
causing as little headache as possible?

My proposal is as follows:

* Let's properly analyze the GCC mapping, just on its own.  Maybe it's 
  broken, if it is then I think we've got a pretty decent argument to 
  just ignore old binary compatibility -- if it was always broken then 
  we can't break it, so who cares.
* Assuming the GCC mapping is internally consistent, let's analyze 
  arbitrary mixes of the GCC and ISA mappings.  It's not generally true 
  that two correct mappings can be freely mixed, but maybe we've got 
  some cases that work fine.  Maybe we even get lucky and everything's 
  compatible, who knows (though I'm worried about the same .aq vs fence 
  stuff we've had in LKMM a few times).
* Assuming there's any issue mixing the GCC and ISA mappings, let's add 
  a flag to GCC.  Something like -mrvwmo-compat={legacy,both,isa}, where:
    - =legacy does what we have now.  We can eventually deprecate this, 
      and assuming =both is fast enough maybe we don't even need it.
    - =both implements a mapping that's compatible with both the GCC and 
      ISA mappings.  This might be slow or something, but it'll be 
      correct with both.  We can set this to the default now, as it's 
      safe.
    - =isa implements the ISA mappings.

Then we can go stick some marker in the ELF saying "this binary is 
compatible with the ISA mappings" to triage binaries in systems.  That 
way distros can decide when they want to move to -mrvwmo-compat=isa by 
default, maybe when they naturally do a world rebuild.  Eventually we 
can make that the default in GCC upstream and later deprecate the 
compatibility modes.

That's going to take a bit of work on the formal memory model side of 
things, but I think we've at least got enough of Andrea's bandwidth to 
make it happen in a reasonable timeframe.  I don't think there's a ton 
of implementation work to do once we sort out the desired mappings, and 
a chunk of that can probably start in parallel as we'll likely need 
stuff like the ELF tagging eventually.

Landing this before stage4 might be tricky, though.  I tell myself we're 
not going to take late backend features every release... ;)