From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg1-x52b.google.com (mail-pg1-x52b.google.com [IPv6:2607:f8b0:4864:20::52b]) by sourceware.org (Postfix) with ESMTPS id B9FC83858C52 for ; Fri, 14 Oct 2022 21:57:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B9FC83858C52 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-pg1-x52b.google.com with SMTP id h185so5426757pgc.10 for ; Fri, 14 Oct 2022 14:57:25 -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=twVD3iZ8KHRvq2j3HjwPn+V4JTxIId9CEnoc4GvfVs4=; b=CBZuaXcVP2qJckX2LZwBPny9AKs+vOiDIziAJF/ppZb9mxMwCa7EUBWj0uh5AdqTPD sVFWtGLQM7zeKWhwEUz4mRpaMS+eSCe1zxWEsAqDrL/pfrbRYh5rqOJQC4VgXJlk5pGa SmWhF7luQXf+96d9K5YBCF9UktLuyhAFLd835h/pq16m9KgsLm//oQF43I/8+5JaOPM0 Q7lm/qe0BqivhG+P3ZiSszpjjgGwS7CSlcul2D6pK68YO9B4Y+xW7s5+iE8W763Q+wvm 4lXK1O51Ch+q5zBfv+hXPvrlbLK6nwihN7e82m/StK/KphT7073yjq+BLnodV1rLZ+el sQ6w== 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=twVD3iZ8KHRvq2j3HjwPn+V4JTxIId9CEnoc4GvfVs4=; b=hSwQ2eOJm6SAYpLb67rgkTxD8tX004ykii7WPhkRsLTN5JmprtTHJRqhM3WP5MNWR/ NUZRu9zTx1DKdr61gzmxCyZk6SBjSMlV1P5iRO6dLHc1Jm3VXwsvuxSiLC0cfZfGPBLk Vydzt1W6Dj3Djh5mkW9C6p+vrP+VstXB6G6RNOF23f2G8wmDNK7wlgv+X3ZvJlrcuN7f VzSwqThArS4ZpTa0w02Ony8KmZaKVx7+gIHC1uNwHsUHvipXAn/DT7BK4YQtPwpqn4vf yHgNDHdPxAw+jUQ/Lsz1jDHojmCUv86O5uUFL3gJy44pYWargr1agHA8V80XJG5qPjmA QWhA== X-Gm-Message-State: ACrzQf2jMUj9ktWNgsEZiWFUQk8ncUk6oK/CfSxE7sKsbjlbqYnBylHa IKjmij/+UZoGY2nsiySyoFpivg== X-Google-Smtp-Source: AMsMyM4uZ0wnaaXnaivCvRd1o4vIREzsvqv+CGE0aTkxDyfTLKcj3Tb/F9pzFvTJZV9PED7QR0pIkw== X-Received: by 2002:a05:6a00:1a47:b0:52e:6a8c:5430 with SMTP id h7-20020a056a001a4700b0052e6a8c5430mr7435425pfv.48.1665784644060; Fri, 14 Oct 2022 14:57:24 -0700 (PDT) Received: from localhost ([50.221.140.188]) by smtp.gmail.com with ESMTPSA id v14-20020a1709028d8e00b00176e2fa216csm2190269plo.52.2022.10.14.14.57.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 14 Oct 2022 14:57:22 -0700 (PDT) Date: Fri, 14 Oct 2022 14:57:22 -0700 (PDT) X-Google-Original-Date: Fri, 14 Oct 2022 14:57:30 PDT (-0700) Subject: Re: [PATCH v2 00/10] [RISC-V] Atomics improvements [PR100265/PR100266] In-Reply-To: <818c3a05-feec-4f0a-8268-48074af14ac6@gmail.com> CC: cmuellner@gcc.gnu.org, gcc-patches@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: jeffreyalaw@gmail.com 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.8 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 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); } $ 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