From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-x42b.google.com (mail-pf1-x42b.google.com [IPv6:2607:f8b0:4864:20::42b]) by sourceware.org (Postfix) with ESMTPS id 93FE63858D38 for ; Sat, 15 Oct 2022 00:31:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 93FE63858D38 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=dabbelt.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=dabbelt.com Received: by mail-pf1-x42b.google.com with SMTP id h13so6320302pfr.7 for ; Fri, 14 Oct 2022 17:31:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dabbelt-com.20210112.gappssmtp.com; s=20210112; h=content-transfer-encoding:mime-version:message-id:to:from:cc :in-reply-to:subject:date:from:to:cc:subject:date:message-id :reply-to; bh=KzXVR1xq/pQ1VPbEtx8T0jKDFwRGOBo3SWyPuNSN4RM=; b=t11KKay4X5SxiqVP7KcA8pV5YvdHgS1rlfITnXu9zZlkODrwVch7Hm6AjSDI8S+nAH Qpizq/sBKoSqG/DNyshnOYlIy+ZXxbyGcy/9j+cB9yx9mHdGRnsNuucX2wODlP/ZQXKL 4EWNof65pr3HNSoqklhrRSA+EoRahN7fzTbnj2c8c+lPK4/SO7z9zSKglfpSabtyUfjO /dqp0mLQUpGzI1yCBIlRLvJWyvCG7Ib4om6iOOHd5tDcVjGMOBKopbiXscyzAaY5vxhY Zb/jYYr8qrKgL3RMnCC0h1AmWNQwlX2u4e6o8Jty+BK3R4muoVK5F1/pJajpfrQNp5ka sDYg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:to:from:cc :in-reply-to:subject:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=KzXVR1xq/pQ1VPbEtx8T0jKDFwRGOBo3SWyPuNSN4RM=; b=EmhDtUHEO0eW+IdUZT9ToHMo22VKuL82yMopRxFoqOt6bFwUQl/0ge5UDKpNbyn+LF 2sQg5leyCyCFDCjOkhiwah6G/jCM7mLEW4UGdRicxmLZmN/e2EI+nJjcHzn2nV/3jeP6 L/KFFuIqlSv/3eWEP/ChaJyd2/BEzWcYOGS+Su7KrRWLpLkMAieE71VXQWRhYJVMFcn8 CVXE7UmPzJe3Dh6I5IjmO3ML/O4+BLHahrgx85mEqVPtOiCYvWMOR6TvwbB+Vz69Fbon ukkWj9bK2TOvMlmChYEOBo8JktkmNgSb6MJtKRqA49JuBprYwFh8txXK4p3i1cBPcPEP w0cw== X-Gm-Message-State: ACrzQf2NY/BRDoox+tu6e8CBY1G8USJZTSMGeLgf9p34jmV7je2l4NvH 1/i1Xhmd1nLdX7VgSotS/dN8Iw== X-Google-Smtp-Source: AMsMyM5OYvY+b4tBTAGld352YqPeUREwAJX2mWlZxJDcV60XFqG5eRQBXJnPF/5JI9jskdhw+l5A4Q== X-Received: by 2002:a63:ed07:0:b0:442:87:3a38 with SMTP id d7-20020a63ed07000000b0044200873a38mr465407pgi.216.1665793870306; Fri, 14 Oct 2022 17:31:10 -0700 (PDT) Received: from localhost ([50.221.140.188]) by smtp.gmail.com with ESMTPSA id e19-20020a170902ed9300b00173411a4385sm2269641plj.43.2022.10.14.17.31.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 14 Oct 2022 17:31:09 -0700 (PDT) Date: Fri, 14 Oct 2022 17:31:09 -0700 (PDT) X-Google-Original-Date: Fri, 14 Oct 2022 17:31:18 PDT (-0700) Subject: Re: [PATCH v2 00/10] [RISC-V] Atomics improvements [PR100265/PR100266] In-Reply-To: CC: jeffreyalaw@gmail.com, cmuellner@gcc.gnu.org, Vineet Gupta , kito.cheng@sifive.com, gnu-toolchain@rivosinc.com, philipp.tomsich@vrull.eu, christoph.muellner@vrull.eu From: Palmer Dabbelt To: gcc-patches@gcc.gnu.org Message-ID: Mime-Version: 1.0 (MHng) Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.7 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Fri, 14 Oct 2022 14:57:22 PDT (-0700), Palmer Dabbelt wrote: > On Fri, 14 Oct 2022 13:39:33 PDT (-0700), jeffreyalaw@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::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 > > bool f(std::atomic& p, long& o, long n) { > return p.compare_exchange_weak(o, n, std::memory_order_acq_rel, std::memory_order_release); Eric points out this is bogus code, the spec forbids release as the fail ordering (which akes sense). Just kind of randomly permutating these ordering arguments ends up with some generated code that seems off, for example release/acq_rel produces lr/sc.rl (and no fences). I don't think any of that is very relevant for the GCC discussion, though, as there's a bunch of other mappings specified by the ISA and we should just sort this one out. > } > > > $ clang-15.0.0 -std=c++17 -O3 > f(std::atomic&, long&, long): # @f(std::atomic&, 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): > 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 > > -- > You received this message because you are subscribed to the Google Groups "gnu-toolchain" group. > To unsubscribe from this group and stop receiving emails from it, send an email to gnu-toolchain+unsubscribe@rivosinc.com. > To view this discussion on the web visit https://groups.google.com/a/rivosinc.com/d/msgid/gnu-toolchain/mhng-580bc331-c4d5-4f04-afd5-30f09f68b660%40palmer-ri-x1c9a. > For more options, visit https://groups.google.com/a/rivosinc.com/d/optout.