Re: PATCH: Add LWP support for upcoming AMD Orochi processor.

[Jakub Jelinek <jakub at redhat dot com>]

On Thu, Dec 10, 2009 at 01:42:41PM -0600, Sebastian Pop wrote:
> > 1) llwpcb* - the builtins are declared void __builtin_ia32_llwpcb* (void),
> > but lwpintrin.h expects them to take void * argument.
> 
> Fixed.
> 
> > If I understand right, the
> > insn in reality has 3 address sizes to support 16-bit/32-bit/64-bit code,
> > I fail to see why we'd need 3 different intrinsics, instead of just one and
> > one builtin that takes void * address and uses the insn matching Pmode.
> 
> Unless I am doing something wrong, I remarked that the HI mode is not
> generated when I factor it in the :P mode.  In the attached patch I
> merged only the 32 and 64 bit modes into one pattern for the llwpcb
> and slwpcb insns.

Why do you want the HI mode version generated, ever (except for 16-bit code which
gcc doesn't emit)?  IMNSHO you don't want to ever use even the 32-bit one
for -m64 code.  I believe you want lwpintrin.h to provide one intrinsic,
not three, it takes a void * argument anyway in all 3 cases.  What would be
an intrinsic that just uses some lower bits from the pointer good for?
Why should a user care whether the pointer is 32-bit or 64-bit or 16-bit?

What could make some very limited sense is when that void * pointer is
initialized through say movl symbol, %edx use the 32-bit insn even for -m64
code to save one byte, but 1) I doubt it is worth writing the peepholes
2) you don't want to let the user make this decision, instead you want the
compiler to decide (if at all).  And 16-bit addresses aren't really useful
at all.

	Jakub