Re: Resend: [PATCH] [MIPS] Fix asm constraints for 'ins' instructions.

[David Daney <ddaney at avtrex dot com>]

Richard Sandiford wrote:
> David Daney <ddaney@avtrex.com> 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@avtrex.com>
>>>> ---
>>>> 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?

David Daney