Date: Thu, 13 Dec 2001 11:27:56 -0800 (PST) From: John Baldwin <jhb@FreeBSD.org> To: Bruce Evans <bde@zeta.org.au> Cc: current@FreeBSD.org Subject: Re: Patch Review: i386 asm cleanups in the kernel Message-ID: <XFMail.011213112756.jhb@FreeBSD.org> In-Reply-To: <20011213215212.O277-100000@gamplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 13-Dec-01 Bruce Evans wrote:
>> --- i386/i386/identcpu.c 30 Nov 2001 11:57:23 -0000 1.96
>> +++ i386/i386/identcpu.c 6 Dec 2001 07:58:25 -0000
>> @@ -115,10 +115,11 @@
>> static void
>> do_cpuid(u_int ax, u_int *p)
>> {
>> +
>> + p[0] = ax;
>> __asm __volatile(
>> "cpuid"
>> - : "=a" (p[0]), "=b" (p[1]), "=c" (p[2]), "=d" (p[3])
>> - : "0" (ax)
>> + : "+a" (p[0]), "=b" (p[1]), "=c" (p[2]), "=d" (p[3])
>> );
>> }
>>
>
> "0" here was bogus. Just replace it by "a".
So use "a" twice and gcc will get that right?
>> - : "=a" (low), "=d" (high) \
>> - : "0" (low), "1" (high) \
>> - : "cx", "bx")
>> + : "+a" (low), "+d" (high) \
>> + : : "ecx", "ebx")
>
> Weren't the register names in the clobber list the normal ones?
I was just being pedantic since we aren't just clobbering the low words of the
registers.
>> @@ -938,7 +935,7 @@
>> "movl 4(%0),%%eax ; adcl %%eax,4(%0)\n\t"
>> "movl 8(%0),%%eax ; adcl %%eax,8(%0)\n\t"
>> "movl 12(%0),%%eax ; adcl %%eax,12(%0)"
>> - ::"r" (c):"ax");
>> + : :"r" (c):"eax", "memory");
>> }
>>
>> static void
>
> This should use output operands c[0]..c[3], not a general "memory" clobber.
Ok.
>> [... more suboptimal "memory" clobbers]
I changed the simple ones, but not the harder ones. (gcc doesn't like having
more than 10 operands, even if they are memory or immediates)
> I wouldn't trust all the little changes to math_emulate.c without runtime
> testing. It won't be tested by using it in normal operation.
Ok. I won't commit it until it is tested.
>> Index: i386/include/bus_pc98.h
>> ===================================================================
>> RCS file: /usr/cvs/src/sys/i386/include/bus_pc98.h,v
>> retrieving revision 1.19
>> diff -u -r1.19 bus_pc98.h
>> --- i386/include/bus_pc98.h 7 Oct 2001 10:04:18 -0000 1.19
>> +++ i386/include/bus_pc98.h 7 Dec 2001 19:10:37 -0000
>> @@ -203,11 +203,10 @@
>> \
>> __asm __volatile("call *%2" \
>> :"=a" (result), \
>> - "=d" (offset) \
>> + "+d" (offset) \
>> :"o" (bsh->bsh_bam.bs_read_##BWN), \
>> - "b" (bsh), \
>> - "1" (offset) \
>> - ); \
>> + "b" (bsh) \
>> + :"ecx","memory"); \
>> \
>> return result; \
>> }
>
> It's surprising that the missing "ecx" in lots of clobber lists in this
> file didn't cause problems.
It depends on what the functions do. If they are written in asm, they might be
preserving temporary registers rather than trashing them. I was just changing
the clobbers to assume that only call-safe registers were preserved.
>> - __asm __volatile("bsfl %0,%0" : "=r" (result) : "0" (mask));
>> + __asm __volatile("bsfl %0,%0" : "+r" (result));
>> return (result);
>> }
>
> "0" here was bogus, except we abuse the same operand for input and output.
> I checked that gcc does the right thing (reuse the input register for
> output if the input operand becomes dead) with a non-hand-optimized version:
>
> __asm __volatile("bsfl %1,%0" : "=r" (result) : "r" (mask));
> ...
> main() { return bsfl(23); }
>
> "rm" (mask) works too.
Ok, that's simpler then.
> The other changes seem to be OK. I checked about 1/4 of them.
Ok, well, I've updated the patch at the same location.
> Bruce
--
John Baldwin <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve!" - http://www.FreeBSD.org/
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?XFMail.011213112756.jhb>