David Daney <ddaney@xxxxxxxxxx> writes:
> Richard Sandiford wrote:
>> David Daney <ddaney@xxxxxxxxxx> writes:
>>> Ralf Baechle wrote:
>>>> On Wed, Jun 11, 2008 at 10:04:25AM -0700, David Daney wrote:
>>>>
>>>>> The third operand to 'ins' must be a constant int, not a register.
>>>>>
>>>>> Signed-off-by: David Daney <ddaney@xxxxxxxxxx>
>>>>> ---
>>>>> include/asm-mips/bitops.h | 6 +++---
>>>>> 1 files changed, 3 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/include/asm-mips/bitops.h b/include/asm-mips/bitops.h
>>>>> index 6427247..9a7274b 100644
>>>>> --- a/include/asm-mips/bitops.h
>>>>> +++ b/include/asm-mips/bitops.h
>>>>> @@ -82,7 +82,7 @@ static inline void set_bit(unsigned long nr, volatile unsigned long *addr)
>>>>> "2: b 1b \n"
>>>>> " .previous \n"
>>>>> : "=&r" (temp), "=m" (*m)
>>>>> - : "ir" (bit), "m" (*m), "r" (~0));
>>>>> + : "i" (bit), "m" (*m), "r" (~0));
>>>>> #endif /* CONFIG_CPU_MIPSR2 */
>>>>> } else if (cpu_has_llsc) {
>>>>> __asm__ __volatile__(
>>>> An old trick to get gcc to do the right thing. Basically at the stage when
>>>> gcc is verifying the constraints it may not yet know that it can optimize
>>>> things into an "i" argument, so compilation may fail if "r" isn't in the
>>>> constraints. However we happen to know that due to the way the code is
>>>> written gcc will always be able to make use of the "i" constraint so no
>>>> code using "r" should ever be created.
>>>>
>>>> The trick is a bit ugly; I think it was used first in asm-i386/io.h ages ago
>>>> and I would be happy if we could get rid of it without creating new problems.
>>>> Maybe a gcc hacker here can tell more?
>>> It is not nice to lie to GCC.
>>>
>>> CCing GCC and Richard in hopes that a wider audience may shed some light on the issue.
>>
>> You _might_ be able to use "i#r" instead of "ri", but I wouldn't
>> really recommend it. Even if it works now, I don't think there's
>> any guarantee it will in future.
>>
>> There are tricks you could pull to detect the problem at compile time
>> rather than assembly time, but that's probably not a big win. And again,
>> I wouldn't recommend them.
>>
>> I'm not saying anything you don't know here, but if the argument is
>> always a syntactic constant, the safest bet would be to apply David's
>> patch and also convert the function into a macro. I notice some other
>> ports use macros rather than inline functions here. I assume you've
>> deliberately rejected macros as being too ugly though.
>
> I am still a little unclear on this.
>
> To restate the question:
>
> static inline void f(unsigned nr, unsigned *p)
> {
> unsigned short bit = nr & 5;
>
> if (__builtin_constant_p(bit)) {
> __asm__ __volatile__ (" foo %0, %1" : "=m" (*p) : "i" (bit));
> }
> else {
> // Do something else.
> }
> }
> .
> .
> .
> f(3, some_pointer);
> .
> .
> .
>
> Among the versions of GCC that can build the current kernel, will any
> fail on this code because the "i" constraint cannot be matched when
> expanded to RTL?
Someone will point this out if I don't, so for avoidance of doubt:
this needs to be always_inline. It also isn't guaranteed to work
with "bit" being a separate statement. I'm not truly sure it's
guaranteed to work even with:
__asm__ __volatile__ (" foo %0, %1" : "=m" (*p) : "i" (nr & 5));
but I think we'd try hard to make sure it does.
I think Maciej said that 3.2 was the minimum current version.
Even with those two issues sorted out, I don't think you can
rely on this sort of thing with compilers that used RTL inlining.
(always_inline does go back to 3.2, in case you're wondering.)
Richard
