Bug 91591

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

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

(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

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.

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.

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

(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

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).

(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.

(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

Created attachment 117615 [details] [review]
patch

Try this -- defines __STDC_LIMIT_MACROS before including limits.h.

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)

Created attachment 117618 [details] [review]
patch

Oh, that's true. Here's a patch to define it globally.

Comment on attachment 117618 [details] [review]
patch

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

Looks great.

Reviewed-by: Jose Fonseca <jfonseca@vmware.com>

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.

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

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>