[Mesa-dev] [PATCH v2] math: Import isinf and others to global namespace

[Jose Fonseca]

I just noticed you initial patch said "(and
glibc >= 2.23)".

I don't have it.  So maybe that's all we need is check __GLIBC__ /
_GLIBC_MINOR__ and only do it then.

Jose



On 13/04/16 15:21, Jose Fonseca wrote:
> This was with GLIBC 2.21 in ubuntu 15.10:
> 
> $ ldd --version
> ldd (Ubuntu GLIBC 2.21-0ubuntu4.1) 2.21
> 
> And it was with GCC 5.0.  I also tried GCC 4.9 but no difference.
> 
> And the funny thing is that I can't repro the nouveau build failure neither.
> 
> _Something_ causes these functions to be available on my system.  Not
> sure exactly what.
> 
> I think we'll probably need to make a small test case for this, and run
> `g++ -E` to get to the bottom of this.
> 
> And if nothing else, we might need to run this test case at configure.ac
> time, to see when we need to do the `using std::isinf;`.
> 
> 
> Jose
> 
> 
> 
> On 13/04/16 12:16, Pierre Moreau wrote:
>> :-( I'll have another look this evening. Which version of glibc did it failed with?
>>
>> (I saw your reply to the other patch, I'll dig deeper this evening.)
>>
>> Thanks!
>> Pierre
>>
>>> On 13 Apr 2016, at 08:22, Jose Fonseca <jfonseca at vmware.com> wrote:
>>>
>>>> On 01/04/16 13:18, Pierre Moreau wrote:
>>>>> On 01:11 PM - Apr 01 2016, Jose Fonseca wrote:
>>>>>> On 31/03/16 23:08, Pierre Moreau wrote:
>>>>>> Starting from C++11, several math functions, like isinf, moved into the std
>>>>>> namespace. Since cmath undefines those functions before redefining them inside
>>>>>> the namespace, and glibc 2.23 defines the C variants as macros, the C variants
>>>>>> in global namespace are not accessible any longer.
>>>>>>
>>>>>> v2: Move the fix outside of Nouveau, as suggested by Jose Fonseca, since anyone
>>>>>>       might need it when GCC switches to C++14 by default with GCC 6.0.
>>>>>>
>>>>>> Signed-off-by: Pierre Moreau <pierre.morrow at free.fr>
>>>>>> ---
>>>>>>    include/cpp11_math.h                | 61 +++++++++++++++++++++++++++++++++++++
>>>>>>    src/gallium/auxiliary/util/u_math.h |  3 ++
>>>>>>    2 files changed, 64 insertions(+)
>>>>>>    create mode 100644 include/cpp11_math.h
>>>>>>
>>>>>> diff --git a/include/cpp11_math.h b/include/cpp11_math.h
>>>>>> new file mode 100644
>>>>>> index 0000000..1f4aa3c
>>>>>> --- /dev/null
>>>>>> +++ b/include/cpp11_math.h
>>>>>
>>>>> I'm not sure a new header is necessary for this.
>>>>>
>>>>> What this is doing is making C++11 math functions "appear" like C99 ones.
>>>>> It's not making things matching C++11 standard.
>>>>>
>>>>> So IMO the right place for this is the end of c99_math.h
>>>>
>>>> I was planning to put it in the c99 header first, but as I was including some
>>>> C++ header, I felt that having a separate header which would only be included
>>>> by C++ files would be better. I could have the `#ifdef __cplusplus` around the
>>>> whole block inside the c99_math.h instead, if you prefer it that way.
>>>>
>>>> Thanks,
>>>> Pierre
>>>
>>> I moved this to c99_math.h and was about to commit, but this causes build failures with gcc 5.0:
>>>
>>>    $ scons
>>>    [...]
>>> In file included from src/gallium/auxiliary/util/u_math.h:44:0,
>>>                   from src/mesa/main/macros.h:35,
>>>                   from src/compiler/glsl/lower_vec_index_to_swizzle.cpp:36:
>>> include/c99_math.h:198:12: error: ‘constexpr bool std::isinf(double)’ conflicts with a previous declaration
>>> using std::isinf;
>>>              ^
>>> In file included from /usr/include/features.h:364:0,
>>>                   from /usr/include/stdio.h:27,
>>>                   from src/compiler/glsl/ir.h:29,
>>>                   from src/compiler/glsl/lower_vec_index_to_swizzle.cpp:32:
>>> /usr/include/x86_64-linux-gnu/bits/mathcalls.h:201:1: note: previous declaration ‘int isinf(double)’
>>> __MATHDECL_1 (int,isinf,, (_Mdouble_ __value)) __attribute__ ((__const__));
>>> ^
>>>
>>>
>>>
>>>
>>> It looks like newer GLIBC already does this somehow..  I'm not sure what's the exact trigger.
>>>
>>>
>>> I wonder if the problem can be avoided by including math.h at the top:
>>>
>>>
>>> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp
>>> index 500ab89..e61f59e7 100644
>>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp
>>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp
>>> @@ -20,6 +20,7 @@
>>>    * OTHER DEALINGS IN THE SOFTWARE.
>>>    */
>>>
>>> +#include <math.h>
>>> #include "codegen/nv50_ir.h"
>>> #include "codegen/nv50_ir_target.h"
>>>
>>>
>>>
>>>
>>>
>>>
>>> Jose
>>>
>>
>