From: Nick Desaulniers <ndesaulniers@google.com> To: Michael Ellerman <mpe@ellerman.id.au> Cc: linuxppc-dev@lists.ozlabs.org, llvm@lists.linux.dev Subject: Re: [PATCH] powerpc/dcr: Use cmplwi instead of 3-argument cmpli Date: Thu, 14 Oct 2021 11:15:09 -0700 [thread overview] Message-ID: <CAKwvOdkcWCFEWxjwPcABA=6qWas+wuVcdDS=T1Oh-j81fYxxFg@mail.gmail.com> (raw) In-Reply-To: <20211014024424.528848-1-mpe@ellerman.id.au> On Wed, Oct 13, 2021 at 7:44 PM Michael Ellerman <mpe@ellerman.id.au> wrote: > > In dcr-low.S we use cmpli with three arguments, instead of four > arguments as defined in the ISA: > > cmpli cr0,r3,1024 > > This appears to be a PPC440-ism, looking at the "PPC440x5 CPU Core > User’s Manual" it shows cmpli having no L field, but implied to be 0 due > to the core being 32-bit. It mentions that the ISA defines four > arguments and recommends using cmplwi. > > dcr-low.S is only built 32-bit, because it is only built when > DCR_NATIVE=y, which is only selected by 40x and 44x. Looking at the > generated code (with gcc/gas) we see cmplwi as expected. > > Although gas is happy with the 3-argument version when building for > 32-bit, the LLVM assembler is not and errors out with: > > arch/powerpc/sysdev/dcr-low.S:27:10: error: invalid operand for instruction > cmpli 0,%r3,1024; ... > ^ > > Switching to the four argument version avoids any confusion when reading > the ISA, fixes the issue with the LLVM assembler, and also means the > code could be built 64-bit in future (though that's very unlikely). Thank you Michael. We've definitely run into a few cases where GAS allowed for various short-hand forms of various instructions (a fair amount of recent work was around 32b ARM and THUMB parity in LLVM). LLVM's assembler is mostly generated from a high level description of the instruction formats, so it's not always as flexible as a hand written parser would be. (There is a mix of hand written arch specific parsing, but most of the parser is arch agnostic, and all of the instruction descriptions are described in an LLVM specific high level language called tablegen which generates C++ that is used by the assembler, but also the disassembler, the compiler, and even the linker if need be). Link: https://github.com/ClangBuiltLinux/linux/issues/1419 Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > Reported-by: Nick Desaulniers <ndesaulniers@google.com> > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> > --- > arch/powerpc/sysdev/dcr-low.S | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/powerpc/sysdev/dcr-low.S b/arch/powerpc/sysdev/dcr-low.S > index efeeb1b885a1..329b9c4ae542 100644 > --- a/arch/powerpc/sysdev/dcr-low.S > +++ b/arch/powerpc/sysdev/dcr-low.S > @@ -11,7 +11,7 @@ > #include <asm/export.h> > > #define DCR_ACCESS_PROLOG(table) \ > - cmpli cr0,r3,1024; \ > + cmplwi cr0,r3,1024; \ > rlwinm r3,r3,4,18,27; \ > lis r5,table@h; \ > ori r5,r5,table@l; \ > -- > 2.25.1 > -- Thanks, ~Nick Desaulniers
WARNING: multiple messages have this Message-ID (diff)
From: Nick Desaulniers <ndesaulniers@google.com> To: Michael Ellerman <mpe@ellerman.id.au> Cc: llvm@lists.linux.dev, linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH] powerpc/dcr: Use cmplwi instead of 3-argument cmpli Date: Thu, 14 Oct 2021 11:15:09 -0700 [thread overview] Message-ID: <CAKwvOdkcWCFEWxjwPcABA=6qWas+wuVcdDS=T1Oh-j81fYxxFg@mail.gmail.com> (raw) In-Reply-To: <20211014024424.528848-1-mpe@ellerman.id.au> On Wed, Oct 13, 2021 at 7:44 PM Michael Ellerman <mpe@ellerman.id.au> wrote: > > In dcr-low.S we use cmpli with three arguments, instead of four > arguments as defined in the ISA: > > cmpli cr0,r3,1024 > > This appears to be a PPC440-ism, looking at the "PPC440x5 CPU Core > User’s Manual" it shows cmpli having no L field, but implied to be 0 due > to the core being 32-bit. It mentions that the ISA defines four > arguments and recommends using cmplwi. > > dcr-low.S is only built 32-bit, because it is only built when > DCR_NATIVE=y, which is only selected by 40x and 44x. Looking at the > generated code (with gcc/gas) we see cmplwi as expected. > > Although gas is happy with the 3-argument version when building for > 32-bit, the LLVM assembler is not and errors out with: > > arch/powerpc/sysdev/dcr-low.S:27:10: error: invalid operand for instruction > cmpli 0,%r3,1024; ... > ^ > > Switching to the four argument version avoids any confusion when reading > the ISA, fixes the issue with the LLVM assembler, and also means the > code could be built 64-bit in future (though that's very unlikely). Thank you Michael. We've definitely run into a few cases where GAS allowed for various short-hand forms of various instructions (a fair amount of recent work was around 32b ARM and THUMB parity in LLVM). LLVM's assembler is mostly generated from a high level description of the instruction formats, so it's not always as flexible as a hand written parser would be. (There is a mix of hand written arch specific parsing, but most of the parser is arch agnostic, and all of the instruction descriptions are described in an LLVM specific high level language called tablegen which generates C++ that is used by the assembler, but also the disassembler, the compiler, and even the linker if need be). Link: https://github.com/ClangBuiltLinux/linux/issues/1419 Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > Reported-by: Nick Desaulniers <ndesaulniers@google.com> > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> > --- > arch/powerpc/sysdev/dcr-low.S | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/powerpc/sysdev/dcr-low.S b/arch/powerpc/sysdev/dcr-low.S > index efeeb1b885a1..329b9c4ae542 100644 > --- a/arch/powerpc/sysdev/dcr-low.S > +++ b/arch/powerpc/sysdev/dcr-low.S > @@ -11,7 +11,7 @@ > #include <asm/export.h> > > #define DCR_ACCESS_PROLOG(table) \ > - cmpli cr0,r3,1024; \ > + cmplwi cr0,r3,1024; \ > rlwinm r3,r3,4,18,27; \ > lis r5,table@h; \ > ori r5,r5,table@l; \ > -- > 2.25.1 > -- Thanks, ~Nick Desaulniers
next prev parent reply other threads:[~2021-10-14 18:15 UTC|newest] Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-10-14 2:44 [PATCH] powerpc/dcr: Use cmplwi instead of 3-argument cmpli Michael Ellerman 2021-10-14 10:56 ` Segher Boessenkool 2021-10-14 18:15 ` Nick Desaulniers [this message] 2021-10-14 18:15 ` Nick Desaulniers 2021-11-02 10:12 ` Michael Ellerman
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='CAKwvOdkcWCFEWxjwPcABA=6qWas+wuVcdDS=T1Oh-j81fYxxFg@mail.gmail.com' \ --to=ndesaulniers@google.com \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=llvm@lists.linux.dev \ --cc=mpe@ellerman.id.au \ /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.