From: Segher Boessenkool <segher@kernel.crashing.org> To: Arnd Bergmann <arnd@arndb.de> Cc: Nathan Chancellor <natechancellor@gmail.com>, Nick Desaulniers <ndesaulniers@google.com>, Michael Ellerman <mpe@ellerman.id.au>, christophe leroy <christophe.leroy@c-s.fr>, 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: workaround clang codegen bug in dcbz Date: Tue, 30 Jul 2019 08:48:56 -0500 [thread overview] Message-ID: <20190730134856.GO31406@gate.crashing.org> (raw) In-Reply-To: <CAK8P3a1GQSyCj1L8fFG4Pah8dr5Lanw=1yuimX1o+53ARzOX+Q@mail.gmail.com> On Tue, Jul 30, 2019 at 09:34:28AM +0200, Arnd Bergmann wrote: > Upon a second look, I think the issue is that the "Z" is an input argument > when it should be an output. clang decides that it can make a copy of the > input and pass that into the inline asm. This is not the most efficient > way, but it seems entirely correct according to the constraints. Most dcb* (and all icb*) do not change the memory pointed to. The memory is an input here, logically as well, and that is obvious. > Changing it to an output "=Z" constraint seems to make it work: > > https://godbolt.org/z/FwEqHf > > Clang still doesn't use the optimum form, but it passes the correct pointer. As I said many times already, LLVM does not seem to treat all asm operands as lvalues. That is a bug. And it is critical for memory operands for example, as should be obvious if you look at at for a few seconds (you pass *that* memory, not a copy of it). The thing you pass has an identity. It's an lvalue. This is true for *all* inline asm operands, not just output operands and memory operands, but it is most obvious there. Or, LLVM might have a bug elsewhere. Either way, the asm is fine, and it has worked fine in GCC since forever. Changing this constraint to be an output constraint would just be obfuscation (we could change *all* operands to *everything* to be inout ("+") constraints, and it won't affect correctness, just the reader's sanity). 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: workaround clang codegen bug in dcbz Date: Tue, 30 Jul 2019 08:48:56 -0500 [thread overview] Message-ID: <20190730134856.GO31406@gate.crashing.org> (raw) In-Reply-To: <CAK8P3a1GQSyCj1L8fFG4Pah8dr5Lanw=1yuimX1o+53ARzOX+Q@mail.gmail.com> On Tue, Jul 30, 2019 at 09:34:28AM +0200, Arnd Bergmann wrote: > Upon a second look, I think the issue is that the "Z" is an input argument > when it should be an output. clang decides that it can make a copy of the > input and pass that into the inline asm. This is not the most efficient > way, but it seems entirely correct according to the constraints. Most dcb* (and all icb*) do not change the memory pointed to. The memory is an input here, logically as well, and that is obvious. > Changing it to an output "=Z" constraint seems to make it work: > > https://godbolt.org/z/FwEqHf > > Clang still doesn't use the optimum form, but it passes the correct pointer. As I said many times already, LLVM does not seem to treat all asm operands as lvalues. That is a bug. And it is critical for memory operands for example, as should be obvious if you look at at for a few seconds (you pass *that* memory, not a copy of it). The thing you pass has an identity. It's an lvalue. This is true for *all* inline asm operands, not just output operands and memory operands, but it is most obvious there. Or, LLVM might have a bug elsewhere. Either way, the asm is fine, and it has worked fine in GCC since forever. Changing this constraint to be an output constraint would just be obfuscation (we could change *all* operands to *everything* to be inout ("+") constraints, and it won't affect correctness, just the reader's sanity). Segher
next prev parent reply other threads:[~2019-07-30 13:49 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 ` [PATCH] powerpc: fix inline asm constraints for dcbz Segher Boessenkool 2019-08-09 21:55 ` Segher Boessenkool 2019-08-09 20:36 ` Nathan Chancellor 2019-08-09 20:36 ` Nathan Chancellor 2019-07-30 13:48 ` Segher Boessenkool [this message] 2019-07-30 13:48 ` [PATCH] powerpc: workaround clang codegen bug in dcbz 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=20190730134856.GO31406@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.