Bug 91591

Summary: rounding.h:102:2: error: #error "Unsupported or undefined LONG_BIT"
Product: Mesa Reporter: Vinson Lee <vlee>
Component: Mesa coreAssignee: Matt Turner <mattst88>
Status: RESOLVED FIXED QA Contact: mesa-dev
Severity: normal    
Priority: medium CC: jfonseca, mattst88
Version: gitKeywords: bisected, regression
Hardware: x86-64 (AMD64)   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Attachments: patch
patch

Description Vinson Lee 2015-08-09 19:40:25 UTC
mesa: 21ccdbdb5dd87b2ee66c4e78b011ec4df29efb98 (master 11.0.0-devel)

Build error on Fedora.

  CC       roundeven_test.o
In file included from roundeven_test.c:30:0:
rounding.h: In function ‘_mesa_lroundevenf’:
rounding.h:102:2: error: #error "Unsupported or undefined LONG_BIT"
 #error "Unsupported or undefined LONG_BIT"
  ^
rounding.h:107:1: warning: no return statement in function returning non-void [-Wreturn-type]
 }
 ^
rounding.h: In function ‘_mesa_lroundeven’:
rounding.h:122:2: error: #error "Unsupported or undefined LONG_BIT"
 #error "Unsupported or undefined LONG_BIT"
  ^
rounding.h:127:1: warning: no return statement in function returning non-void [-Wreturn-type]
 }
 ^
Comment 1 Vinson Lee 2015-08-09 20:13:28 UTC
21ccdbdb5dd87b2ee66c4e78b011ec4df29efb98 is the first bad commit
commit 21ccdbdb5dd87b2ee66c4e78b011ec4df29efb98
Author: Jose Fonseca <jfonseca@vmware.com>
Date:   Sun Aug 9 11:25:41 2015 +0100

    util: Cope with LONG_BIT not being defined on Windows.
    
    Neither MSVC nor MinGW defines LONG_BIT.  For MSVC this was not a problem as
    it doesn't define __x86_64__ macro (it's GCC specific.)
    
    However on Windows long type is guaranteed to be 32bits.
    
    Also add an #error, as GCC will just warn, not throw any error, when no
    value is returned.
    
    Trivial.

:040000 040000 350b7ab0ecfaf0f65abf3a25ebf014bfa025ddcf c3c1bc37290d1e52e77cc05a78d9ee8c35747ce1 M	src
bisect run success
Comment 2 Jose Fonseca 2015-08-09 20:27:38 UTC
My change only makes an existing bug surface.

Even without my change I believe the warning 

  rounding.h:107:1: warning: no return statement in function returning non-void [-Wreturn-type]

was probably there, ever since Matt's commit http://cgit.freedesktop.org/mesa/mesa/commit/?id=680de24545d23d0c2b699020267ca484f81a04a9 .  But because it was not fatal, it was easy to miss.



As I said to Matt on mesa-dev ML, it sounds like LONG_BIT is not standard.  MinGW/MSVC at least don't define it.  It looks like not all Linux define it neither. 

There was a discussion about on Stack Overflow, but is not very conclusive.


On my system, LONG_BIT is defined in 

  /usr/include/x86_64-linux-gnu/bits/xopen_lim.h

And /usr/include/limits.h includes it, but not unconditionally:

  #ifdef	__USE_XOPEN
  # include <bits/xopen_lim.h>
  #endif
Comment 3 Jose Fonseca 2015-08-09 20:28:44 UTC
(In reply to Jose Fonseca from comment #2)
> There was a discussion about on Stack Overflow, but is not very conclusive.

Forgot to paste the link:

  http://stackoverflow.com/questions/15817380/long-bit-in-limits-h
Comment 4 Alexander von Gluck 2015-08-09 20:59:26 UTC
LONG_BIT really isn't standard at all. It isn't defined here on ArchLinux nor is it defined on Haiku.

I've seen mentions of doing the following:
  CHAR_BIT * sizeof(long)


  Archiving build/haiku-x86_64-debug/gallium/auxiliary/libgallium.a ...
In file included from src/mesa/main/macros.h:36:0,
                 from src/util/register_allocate.c:77:
src/util/rounding.h: In function '_mesa_lroundevenf':
src/util/rounding.h:102:2: error: #error "Unsupported or undefined LONG_BIT"
 #error "Unsupported or undefined LONG_BIT"
  ^
src/util/rounding.h:107:1: warning: no return statement in function returning non-void [-Wreturn-type]
 }
 ^
src/util/rounding.h: In function '_mesa_lroundeven':
src/util/rounding.h:122:2: error: #error "Unsupported or undefined LONG_BIT"
 #error "Unsupported or undefined LONG_BIT"
  ^
src/util/rounding.h:127:1: warning: no return statement in function returning non-void [-Wreturn-type]
 }
 ^
  Compiling src/util/strtod.c ...
scons: *** [build/haiku-x86_64-debug/util/register_allocate.os] Error 1
  Generating build/haiku-x86_64-debug/util/format_srgb.c ...
  Indexing build/haiku-x86_64-debug/gallium/auxiliary/libgallium.a ...
scons: building terminated because of errors.
Comment 5 Jose Fonseca 2015-08-09 21:42:10 UTC
There's a potential fix out for review on http://lists.freedesktop.org/archives/mesa-dev/2015-August/091241.html

I only tested on Ubuntu.  If others could verify on others Linux flavors it would be great.
Comment 6 Vinson Lee 2015-08-09 23:54:41 UTC
mesa: 1eaa29cb300e927409281ef0a9413072766eaa3d (11.0.0-devel)

Build failure on CentOS 7.

  CXX    glsl_types.lo
In file included from ../../src/mesa/main/macros.h:36,
                 from ../../src/mesa/main/core.h:45,
                 from glsl_types.cpp:25:
../../src/util/rounding.h:105:2: error: #error "Unsupported long size"
../../src/util/rounding.h:125:2: error: #error "Unsupported long size"
In file included from ../../src/mesa/main/macros.h:36,
                 from ../../src/mesa/main/core.h:45,
                 from glsl_types.cpp:25:
../../src/util/rounding.h: In function ‘long int _mesa_lroundevenf(float)’:
../../src/util/rounding.h:110: warning: no return statement in function returning non-void
../../src/util/rounding.h: In function ‘long int _mesa_lroundeven(double)’:
../../src/util/rounding.h:130: warning: no return statement in function returning non-void
Comment 7 Jose Fonseca 2015-08-10 07:08:46 UTC
(In reply to Vinson Lee from comment #6)
> mesa: 1eaa29cb300e927409281ef0a9413072766eaa3d (11.0.0-devel)

That's my fix. So even with it it fails...


Not sure what do do next:
- replace C-pp logic with regular 
 
   if (sizeof(long) == 8)

- add a configure check for long size. 
- avoid longs
Comment 8 Roland Scheidegger 2015-08-10 14:46:38 UTC
What a mess...
I found some reference that for c++ you need to define __STDC_LIMIT_MACROS before including <stdint.h> to make it work. Dunno though if that's the problem here. Also, do we need to include <limits.h>?
Otherwise I'd just say we should ditch the longs. Not worth the trouble for something every caller just converts to an int anyway (even if it means that indeed _mesa_lroundevenf() wouldn't quite be lrintf equivalent, but who cares).
Comment 9 Jose Fonseca 2015-08-10 15:50:06 UTC
(In reply to Roland Scheidegger from comment #8)
> I found some reference that for c++ you need to define __STDC_LIMIT_MACROS
> before including <stdint.h> to make it work. Dunno though if that's the
> problem here.

We unconditionally define __STDC_LIMIT_MACROS on scons builds.
But we don't on autotools.  LLVM requires that define, so build with LLVM will pick it up anyawy.

Anwyay, roundeven_test.c is a C file, so I don't believe that's the problem.
Comment 10 Roland Scheidegger 2015-08-10 16:15:09 UTC
(In reply to Jose Fonseca from comment #9)
> Anwyay, roundeven_test.c is a C file, so I don't believe that's the problem.

Well the last build failure showed a failure with glsl_types.cpp - only the previous failure was with roundeven_test.c
Comment 11 Matt Turner 2015-08-10 16:23:26 UTC
Created attachment 117615 [details] [review]
patch

Try this -- defines __STDC_LIMIT_MACROS before including limits.h.
Comment 12 Jose Fonseca 2015-08-10 17:45:37 UTC
if __STDC_LIMIT_MACROS is indeed the problem, then it must be defined globally in configure.ac.

Defining in rounding.h won't be effective if by any reason limits.h is included before rounding.h is (ie, the the C file includes limits.h or includes another header that includes limits.h before rounding.h)
Comment 13 Matt Turner 2015-08-10 17:56:26 UTC
Created attachment 117618 [details] [review]
patch

Oh, that's true. Here's a patch to define it globally.
Comment 14 Jose Fonseca 2015-08-10 18:19:05 UTC
Comment on attachment 117618 [details] [review]
patch

Review of attachment 117618 [details] [review]:
-----------------------------------------------------------------

Looks great.

Reviewed-by: Jose Fonseca <jfonseca@vmware.com>
Comment 15 Matt Turner 2015-08-11 22:19:14 UTC
Thanks. Pushed as

commit 02a4fe22b137d4bc8378bedd8319109fd23a50e3
Author: Matt Turner <mattst88@gmail.com>
Date:   Tue Aug 11 15:21:03 2015 -0700

    configure.ac: Always define __STDC_LIMIT_MACROS.
Comment 16 Vinson Lee 2015-08-11 23:15:58 UTC
mesa: 02a4fe22b137d4bc8378bedd8319109fd23a50e3 (master 11.0.0-devel)

SCons build without LLVM is still broken.


  Compiling src/mesa/main/ff_fragment_shader.cpp ...
In file included from src/mesa/main/macros.h:36,
                 from src/mesa/main/ff_fragment_shader.cpp:34:
src/util/rounding.h:105:2: error: #error "Unsupported long size"
src/util/rounding.h:125:2: error: #error "Unsupported long size"
In file included from src/mesa/main/macros.h:36,
                 from src/mesa/main/ff_fragment_shader.cpp:34:
src/util/rounding.h: In function ‘long int _mesa_lroundevenf(float)’:
src/util/rounding.h:110: warning: no return statement in function returning non-void
src/util/rounding.h: In function ‘long int _mesa_lroundeven(double)’:
src/util/rounding.h:130: warning: no return statement in function returning non-void
Comment 17 Vinson Lee 2015-09-28 00:49:05 UTC
commit ee113bbbc51f7c19da5c873410fadabfdd4d4a6d
Author: Vinson Lee <vlee@freedesktop.org>
Date:   Fri Aug 14 15:19:49 2015 -0700

    scons: Always define __STDC_LIMIT_MACROS.
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91591
    Signed-off-by: Vinson Lee <vlee@freedesktop.org>
    Reviewed-by: Roland Scheidegger <sroland@vmware.com>

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.