From: Segher Boessenkool <segher@kernel.crashing.org> To: Arnd Bergmann <arnd@arndb.de> Cc: Nick Desaulniers <ndesaulniers@google.com>, Michael Ellerman <mpe@ellerman.id.au>, christophe leroy <christophe.leroy@c-s.fr>, Nathan Chancellor <natechancellor@gmail.com>, kbuild test robot <lkp@intel.com>, Benjamin Herrenschmidt <benh@kernel.crashing.org>, Paul Mackerras <paulus@samba.org>, linuxppc-dev <linuxppc-dev@lists.ozlabs.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, clang-built-linux <clang-built-linux@googlegroups.com> Subject: Re: [PATCH] powerpc: fix inline asm constraints for dcbz Date: Fri, 9 Aug 2019 16:55:32 -0500 [thread overview] Message-ID: <20190809215531.GN31406@gate.crashing.org> (raw) In-Reply-To: <CAK8P3a3LynWTbpV8=VPm2TqgNM2MnoEyCPJd0PL2D+tcZqJgHg@mail.gmail.com> On Fri, Aug 09, 2019 at 08:28:19PM +0200, Arnd Bergmann wrote: > On Fri, Aug 9, 2019 at 8:21 PM 'Nick Desaulniers' via Clang Built > Linux <clang-built-linux@googlegroups.com> wrote: > > > static inline void dcbz(void *addr) > > { > > - __asm__ __volatile__ ("dcbz %y0" : : "Z"(*(u8 *)addr) : "memory"); > > + __asm__ __volatile__ ("dcbz %y0" : "=Z"(*(u8 *)addr) :: "memory"); > > } > > > > static inline void dcbi(void *addr) > > { > > - __asm__ __volatile__ ("dcbi %y0" : : "Z"(*(u8 *)addr) : "memory"); > > + __asm__ __volatile__ ("dcbi %y0" : "=Z"(*(u8 *)addr) :: "memory"); > > } > > I think the result of the discussion was that an output argument only kind-of > makes sense for dcbz, but for the others it's really an input, and clang is > wrong in the way it handles the "Z" constraint by making a copy, which it > doesn't do for "m". Yes. And clang has probably miscompiled this in all kernels since we have used "Z" for the first time, in 2008 (0f3d6bcd391b). It is not necessarily fatal or at least not easily visible for the I/O accessors: it "just" gets memory ordering wrong slightly (it looks like it does the sync;tw;isync thing around an extra stack access, after it has performed the actual I/O as any other memory load, without any synchronisation). > I'm not sure whether it's correct to use "m" instead of "Z" here, which > would be a better workaround if that works. More importantly though, > clang really needs to be fixed to handle "Z" correctly. "m" allows offset addressing, which these insns do not. That is the same reason you need the "y" output modifier. "m" is wrong here. We have other memory constraints, but do those work with LLVM? Segher
WARNING: multiple messages have this Message-ID (diff)
From: Segher Boessenkool <segher@kernel.crashing.org> To: Arnd Bergmann <arnd@arndb.de> Cc: kbuild test robot <lkp@intel.com>, Nick Desaulniers <ndesaulniers@google.com>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, clang-built-linux <clang-built-linux@googlegroups.com>, Paul Mackerras <paulus@samba.org>, Nathan Chancellor <natechancellor@gmail.com>, linuxppc-dev <linuxppc-dev@lists.ozlabs.org> Subject: Re: [PATCH] powerpc: fix inline asm constraints for dcbz Date: Fri, 9 Aug 2019 16:55:32 -0500 [thread overview] Message-ID: <20190809215531.GN31406@gate.crashing.org> (raw) In-Reply-To: <CAK8P3a3LynWTbpV8=VPm2TqgNM2MnoEyCPJd0PL2D+tcZqJgHg@mail.gmail.com> On Fri, Aug 09, 2019 at 08:28:19PM +0200, Arnd Bergmann wrote: > On Fri, Aug 9, 2019 at 8:21 PM 'Nick Desaulniers' via Clang Built > Linux <clang-built-linux@googlegroups.com> wrote: > > > static inline void dcbz(void *addr) > > { > > - __asm__ __volatile__ ("dcbz %y0" : : "Z"(*(u8 *)addr) : "memory"); > > + __asm__ __volatile__ ("dcbz %y0" : "=Z"(*(u8 *)addr) :: "memory"); > > } > > > > static inline void dcbi(void *addr) > > { > > - __asm__ __volatile__ ("dcbi %y0" : : "Z"(*(u8 *)addr) : "memory"); > > + __asm__ __volatile__ ("dcbi %y0" : "=Z"(*(u8 *)addr) :: "memory"); > > } > > I think the result of the discussion was that an output argument only kind-of > makes sense for dcbz, but for the others it's really an input, and clang is > wrong in the way it handles the "Z" constraint by making a copy, which it > doesn't do for "m". Yes. And clang has probably miscompiled this in all kernels since we have used "Z" for the first time, in 2008 (0f3d6bcd391b). It is not necessarily fatal or at least not easily visible for the I/O accessors: it "just" gets memory ordering wrong slightly (it looks like it does the sync;tw;isync thing around an extra stack access, after it has performed the actual I/O as any other memory load, without any synchronisation). > I'm not sure whether it's correct to use "m" instead of "Z" here, which > would be a better workaround if that works. More importantly though, > clang really needs to be fixed to handle "Z" correctly. "m" allows offset addressing, which these insns do not. That is the same reason you need the "y" output modifier. "m" is wrong here. We have other memory constraints, but do those work with LLVM? Segher
next prev parent reply other threads:[~2019-08-09 21:56 UTC|newest] Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-07-29 20:25 [PATCH] powerpc: workaround clang codegen bug in dcbz Nick Desaulniers 2019-07-29 20:25 ` Nick Desaulniers 2019-07-29 20:32 ` Nathan Chancellor 2019-07-29 20:32 ` Nathan Chancellor 2019-07-29 20:45 ` Nick Desaulniers 2019-07-29 20:45 ` Nick Desaulniers 2019-07-29 20:47 ` Nathan Chancellor 2019-07-29 20:47 ` Nathan Chancellor 2019-07-29 20:49 ` Nick Desaulniers 2019-07-29 20:49 ` Nick Desaulniers 2019-07-29 21:52 ` Segher Boessenkool 2019-07-29 21:52 ` Segher Boessenkool 2019-07-30 7:34 ` Arnd Bergmann 2019-07-30 7:34 ` Arnd Bergmann 2019-07-30 11:17 ` Michael Ellerman 2019-07-30 11:17 ` Michael Ellerman 2019-08-09 18:21 ` [PATCH] powerpc: fix inline asm constraints for dcbz Nick Desaulniers 2019-08-09 18:21 ` Nick Desaulniers 2019-08-09 18:28 ` Arnd Bergmann 2019-08-09 18:28 ` Arnd Bergmann 2019-08-09 20:03 ` Christophe Leroy 2019-08-09 20:03 ` Christophe Leroy 2019-08-09 20:12 ` Arnd Bergmann 2019-08-09 20:12 ` Arnd Bergmann 2019-08-09 22:03 ` Nick Desaulniers 2019-08-09 22:03 ` Nick Desaulniers 2019-08-09 22:10 ` Segher Boessenkool 2019-08-09 22:10 ` Segher Boessenkool 2019-08-09 22:00 ` Segher Boessenkool 2019-08-09 22:00 ` Segher Boessenkool 2019-08-09 22:03 ` [PATCH v3] Revert "powerpc: slightly improve cache helpers" Nick Desaulniers 2019-08-09 22:03 ` Nick Desaulniers 2019-08-10 9:09 ` Michael Ellerman 2019-08-10 9:09 ` Michael Ellerman 2019-08-09 21:55 ` Segher Boessenkool [this message] 2019-08-09 21:55 ` [PATCH] powerpc: fix inline asm constraints for dcbz Segher Boessenkool 2019-08-09 20:36 ` Nathan Chancellor 2019-08-09 20:36 ` Nathan Chancellor 2019-07-30 13:48 ` [PATCH] powerpc: workaround clang codegen bug in dcbz Segher Boessenkool 2019-07-30 13:48 ` Segher Boessenkool 2019-07-30 14:30 ` Arnd Bergmann 2019-07-30 14:30 ` Arnd Bergmann 2019-07-30 16:16 ` Segher Boessenkool 2019-07-30 16:16 ` Segher Boessenkool 2019-07-30 17:07 ` Segher Boessenkool 2019-07-30 18:24 ` Arnd Bergmann 2019-07-30 18:24 ` Arnd Bergmann 2019-07-30 18:24 ` Arnd Bergmann 2019-07-30 19:35 ` Segher Boessenkool 2019-07-30 19:35 ` Segher Boessenkool 2019-07-30 5:31 ` Christophe Leroy 2019-07-30 5:31 ` Christophe Leroy
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190809215531.GN31406@gate.crashing.org \ --to=segher@kernel.crashing.org \ --cc=arnd@arndb.de \ --cc=benh@kernel.crashing.org \ --cc=christophe.leroy@c-s.fr \ --cc=clang-built-linux@googlegroups.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=lkp@intel.com \ --cc=mpe@ellerman.id.au \ --cc=natechancellor@gmail.com \ --cc=ndesaulniers@google.com \ --cc=paulus@samba.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.