Re: [PATCH] powerpc/dcr: Use cmplwi instead of 3-argument cmpli

From: Segher Boessenkool <segher@kernel.crashing.org>
To: Michael Ellerman <mpe@ellerman.id.au>
Cc: ndesaulniers@google.com, linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH] powerpc/dcr: Use cmplwi instead of 3-argument cmpli
Date: Thu, 14 Oct 2021 05:56:43 -0500	[thread overview]
Message-ID: <20211014105643.GF614@gate.crashing.org> (raw)
In-Reply-To: <20211014024424.528848-1-mpe@ellerman.id.au>

Hi!

On Thu, Oct 14, 2021 at 01:44:24PM +1100, Michael Ellerman 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.

It also corresponds to the old POWER instruction set, which had no L
field there, a reserved bit instead.  It used to be that -many allowed
these insns as well, but not anymore.

> Although gas is happy with the 3-argument version when building for
> 32-bit, the LLVM assembler is not and errors out with:

A GAS targeting powerpc64 isn't happy either, fwiw.

>   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).

You are actually now using to the extended opcode cmpwli (a much better
plan :-) )

Thanks,

Segher