Re: [PATCH v3 11/19] RISC-V: Generic <math.h> and soft-fp Routines

[Darius Rad <darius at bluespec dot com>]

On 01/01/2018 10:02 AM, Joseph Myers wrote:
> On Tue, 26 Dec 2017, Palmer Dabbelt wrote:
> 
>> +    FE_INVALID   =
>> +#define FE_INVALID	(0x10)
>> +      FE_INVALID,
>> +  };
> 
> C90 / C++98 don't support trailing commas in enum definitions, and I don't 
> think that's one of the extensions beyond those standards that we intend 
> to require for compilers to use glibc headers (unlike long long, anonymous 
> structs / unions, and probably alignment attributes and asm redirection of 
> functions in certain cases).  Though I see such trailing commas have crept 
> into some other architectures' enum definitions; we don't have any good 
> testing for headers only using the intended features with anything else 
> being appropriately conditioned on compiler support.
> 

Thanks, I'll remove them.

>> +    FE_UPWARD     =
>> +#define FE_UPWARD	(0x3)
>> +      FE_UPWARD,
> 
> Likewise.
> 
>> +# define _FCLASS(x) ({ int res; \
>> +  if (sizeof (x) * 8 > __riscv_flen) __builtin_trap (); \
>> +  if (sizeof (x) == 4) asm ("fclass.s %0, %1" : "=r" (res) : "f" (x)); \
>> +  else if (sizeof (x) == 8) asm ("fclass.d %0, %1" : "=r" (res) : "f" (x)); \
>> +  else __builtin_trap (); \
>> +  res; })
> 
> If this is meant to be a public interface for users of fpu_control.h, then 
> (a) you need to use __res not plain res for the variable used in this 
> macro, and (b) it needs to use (__extension__ ({ ... })).  If it's only 
> intended for use within glibc then it should go in an internal header, not 
> an installed one.
> 

Ok, thanks.  I think we want it public, so I'll make the necessary changes.