On 12/14/2011 10:25 AM, Richard Earnshaw wrote:
> On 10/12/11 23:21, Richard Henderson wrote:
>> diff --git a/libitm/config/arm/hwcap.h b/libitm/config/arm/hwcap.h
>> new file mode 100644
>> index 0000000..16e4034
>> --- /dev/null
>> +++ b/libitm/config/arm/hwcap.h
>> +
>> +/* The following must match the kernel's <asm/procinfo.h>. */
>> +#define HWCAP_ARM_SWP 1
>
> All of these defines are linux-specific. Shouldn't they be wrapped as such?
Well, we need *some* values for other targets. Shouldn't they match, failing
some other useful value to put in?
> That can't be right. The first instruction loads from your literal
> block an address difference, then your second instruction tries to use
> that as an address.
Yes, that's fixed in the followup that's been posed, wrt movw+movt.
> Why don't you just write
>
> .macro ldaddr reg, addr
> ldr \reg, =(\addr - 99b + PC_OFFS)
> 99: add \reg, \reg, pc
> .endm
That was the first thing I tried. Unfortunately, the assembler doesn't accept
arbitrary expressions. That's arguably a bug...
>> + /* Store the VFP registers. Don't use VFP instructions directly
>> + because this code is used in non-VFP multilibs. */
>> + tst r2, #HWCAP_ARM_VFP
>> + it ne
>> + stcne p11, cr8, [sp], {16} /* vstmne sp, {d8-d15} */
>
> You shouldn't rely on the conditional STC instruction not trapping if
> the condition test fails. Conditionally NOT executing an instruction is
> implementation defined if the instruction itself is undefined. Just
> branch round it as you've done for the other instructions below.
Fair enough.
>> + ldm r1, { r4-r11, sp, pc }
>
> Isn't this buffer on the stack?
No. We passed the buffer in on the stack, but we subsequently copied it
elsewhere.
r~
