[PATCH 1/2] powerpc/sstep: Add emulation support for ‘setb’ instruction

Gabriel Paubert paubert at iram.es
Fri Apr 23 20:26:57 AEST 2021


On Thu, Apr 22, 2021 at 06:26:16PM -0500, Segher Boessenkool wrote:
> Hi!
> 
> On Fri, Apr 23, 2021 at 12:16:18AM +0200, Gabriel Paubert wrote:
> > On Thu, Apr 22, 2021 at 02:13:34PM -0500, Segher Boessenkool wrote:
> > > On Fri, Apr 16, 2021 at 05:44:52PM +1000, Daniel Axtens wrote:
> > > > Sathvika Vasireddy <sathvika at linux.vnet.ibm.com> writes:
> > > > > +			if ((regs->ccr) & (1 << (31 - ra)))
> > > > > +				op->val = -1;
> > > > > +			else if ((regs->ccr) & (1 << (30 - ra)))
> > > > > +				op->val = 1;
> > > > > +			else
> > > > > +				op->val = 0;
> > > 
> > > It imo is clearer if written
> > > 
> > > 			if ((regs->ccr << ra) & 0x80000000)
> > > 				op->val = -1;
> > > 			else if ((regs->ccr << ra) & 0x40000000)
> > > 				op->val = 1;
> > > 			else
> > > 				op->val = 0;
> > > 
> > > but I guess not everyone agrees :-)
> > 
> > But this can be made jump free :-):
> > 
> > 	int tmp = regs->ccr << ra;
> > 	op->val = (tmp >> 31) | ((tmp >> 30) & 1);
> 
> The compiler will do so automatically (or think of some better way to
> get the same result); in source code, what matters most is readability,
> or clarity in general (also clarity to the compiler).

I just did a test (trivial code attached) and the original code always
produces one conditional branch at -O2, at least with the cross-compiler
I have on Debian (gcc 8.3). I have tested both -m32 and -m64. The 64 bit
version produces an unnecessary "extsw", so I wrote the second version
splitting the setting of the return value which gets rid of it.

The second "if" is fairly simple to optimize and the compiler does it
properly.

Of course with my suggestion the compiler does not produce any branch. 
But it needs a really good comment.


> 
> (Right shifts of negative numbers are implementation-defined in C,
> fwiw -- but work like you expect in GCC).

Well, I'm not worried about it, since I'd expect a compiler that does
logical right shifts on signed valued to break so much code that it
would be easily noticed (also in the kernel).


> 
> > (IIRC the srawi instruction sign-extends its result to 64 bits).
> 
> If you consider it to work on 32-bit inputs, yeah, that is a simple way
> to express it.
> 
> > > > CR field:      7    6    5    4    3    2    1    0
> > > > bit:          0123 0123 0123 0123 0123 0123 0123 0123
> > > > normal bit #: 0.....................................31
> > > > ibm bit #:   31.....................................0
> > > 
> > > The bit numbers in CR fields are *always* numbered left-to-right.  I
> > > have never seen anyone use LE for it, anyway.
> > > 
> > > Also, even people who write LE have the bigger end on the left normally
> > > (they just write some things right-to-left, and other things
> > > left-to-right).
> > 
> > Around 1985, I had a documentation for the the National's 32032
> > (little-endian) processor family, and all the instruction encodings were
> > presented with the LSB on the left and MSB on the right.
> 
> Ouch!  Did they write "regular" numbers with the least significant digit
> on the left as well?

No, they were not that sadistic!

At least instructions were a whole number of bytes, unlike the iAPX432
where jumps needed to encode target addresses down to the bit level.

> 
> > BTW on these processors, the immediate operands and the offsets (1, 2 or
> > 4 bytes) for the addressing modes were encoded in big-endian byte order,
> > but I digress. Consistency is overrated ;-)
> 
> Inconsistency is the spice of life, yeah :-)

;-)

	Gabriel

 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: setb.c
Type: text/x-csrc
Size: 439 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/linuxppc-dev/attachments/20210423/e2b1a9fe/attachment.c>


More information about the Linuxppc-dev mailing list