Re: [Patch] Fix __mips16 undef macro warnings.

["Carlos O'Donell" <carlos at redhat dot com>]

On 04/29/2014 02:07 PM, Steve Ellcey wrote:
> 
> This is a change to stdlib/longlong.h to fix the undefined macro
> warning for __mips16.  The GCC compiler defines __mips16 when
> compiling in mips16 mode and does not define it othewise.  So
> this patch just changes the #if check to a #if defined().

This is not correct.

We should never add `defined' or `ifdef' to fix this problem
since it puts us back in the same position. We want to remove
all ifdef or defined where possible.

A better use is like this:

* In a central mips header:

/* Long description about this coming from the compiler and
   indicating that it will not be defined when not compiling
   MIPS16.  */
#ifdef __mips16
#define __glibc_mips16 1
#else
#define __glibc_mips16 0
#endif

* Then all uses are always `if __glibc_mips16' to use when
  we want mips16 code, and that macro is always either 0 or 1
  exactly.

Thus you have a single check for the compiler define, and good
comments explaining when it's defined and why, and then a macro
that is either 1 or 0 for that value to use internally.

However, if this is the *only* use of __mips16, then you can
get away with using defined, but you should add a big and
descriptive comment about when and how __mips16 is defined
and set by the compiler.

> stdlib/longlong.h is also in the GCC and binutils trees as
> include/longlong.h.  If this patch is approved I will submit
> it to those groups for checkin as well.  Right now it looks
> like the GCC and glibc files are in sync but the last change
> for clz on ARM did not make it into binutils for some reason.

That's the wrong way around IIRC. The canonical source is gcc,
and binutils and glibc get updated from that.

> Tested with mips-mti-linux-gnu to verify that no object
> files were changed due to this patch.
> 
> OK to checkin?

No.

> Steve Ellcey
> sellcey@mips.com
> 
> 
> 2014-04-29  Steve Ellcey  <sellcey@mips.com>
> 
> 	* stdlib/longlong.h: Use 'defined()' to check __mips16.
> 
> 
> diff --git a/stdlib/longlong.h b/stdlib/longlong.h
> index d45dbe2..070b40c 100644
> --- a/stdlib/longlong.h
> +++ b/stdlib/longlong.h
> @@ -848,7 +848,7 @@ extern UDItype __umulsidi3 (USItype, USItype);
>  #define UMUL_TIME 10
>  #define UDIV_TIME 100
>  
> -#if (__mips == 32 || __mips == 64) && ! __mips16
> +#if (__mips == 32 || __mips == 64) && ! defined (__mips16)
>  #define count_leading_zeros(COUNT,X)	((COUNT) = __builtin_clz (X))
>  #define COUNT_LEADING_ZEROS_0 32
>  #endif
> 

Cheers,
Carlos.