linux-mips
[Top] [All Lists]

Re: [PATCH] MIPS: Handle removal of 'h' constraint in GCC 4.4

To: David Daney <ddaney@caviumnetworks.com>
Subject: Re: [PATCH] MIPS: Handle removal of 'h' constraint in GCC 4.4
From: "Maciej W. Rozycki" <macro@linux-mips.org>
Date: Wed, 25 Feb 2009 01:25:07 +0000 (GMT)
Cc: linux-mips@linux-mips.org, Ralf Baechle <ralf@linux-mips.org>
In-reply-to: <1235516662-24919-1-git-send-email-ddaney@caviumnetworks.com>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <1235516662-24919-1-git-send-email-ddaney@caviumnetworks.com>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Alpine 1.10 (LFD 962 2008-03-14)
On Tue, 24 Feb 2009, David Daney wrote:

> @@ -16,4 +16,11 @@
>  #define GCC_REG_ACCUM "accum"
>  #endif
>  
> +#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 4)
> +#define GCC_NO_H_CONSTRAINT
> +#ifdef CONFIG_64BIT
> +typedef unsigned int uint128_t __attribute__((mode(TI)));
> +#endif
> +#endif
> +
>  #endif /* _ASM_COMPILER_H */

 I suggest to call it u128 in line with all the other such types (and 
place it in <asm/types.h>?).  Also it might be more reasonable to call the 
macro something like GCC_HAS_EXT64_MULT or suchlike as in my opinion we 
should prefer using C code for a calculation like this even if the "h" 
constraint still worked.  We might want to have i128 too. :)

> @@ -62,8 +62,9 @@ static inline void __delay(unsigned long loops)
>  
>  static inline void __udelay(unsigned long usecs, unsigned long lpj)
>  {
> +#ifndef GCC_NO_H_CONSTRAINT
>       unsigned long hi, lo;
> -
> +#endif
>       /*
>        * The rates of 128 is rounded wrongly by the catchall case
>        * for 64-bit.  Excessive precission?  Probably ...
> @@ -77,6 +78,12 @@ static inline void __udelay(unsigned long usecs, unsigned 
> long lpj)
>                                  0x80000000ULL) >> 32);
>  #endif
>  
> +#ifdef GCC_NO_H_CONSTRAINT
> +     if (sizeof(long) == 4)
> +             usecs = ((u64)usecs * lpj) >> 32;
> +     else
> +             usecs = ((uint128_t)usecs * lpj) >> 64;
> +#else
>       if (sizeof(long) == 4)
>               __asm__("multu\t%2, %3"
>               : "=h" (usecs), "=l" (lo)
> @@ -92,6 +99,7 @@ static inline void __udelay(unsigned long usecs, unsigned 
> long lpj)
>               : "=r" (usecs), "=h" (hi), "=l" (lo)
>               : "r" (usecs), "r" (lpj)
>               : GCC_REG_ACCUM);
> +#endif
>  
>       __delay(usecs);
>  }

 Ouch, this is horribly ugly.  It begs for making the conditional chunks a 
pair of separate static inline functions.  You could then do something 
like __delay(__usecs_to_loops(usecs, lpj)) and simply have two variants of 
__usecs_to_loops() with no need to chop code with #ifdefs in the middle.

  Maciej

<Prev in Thread] Current Thread [Next in Thread>