Bug 94383

Created attachment 122107 [details] [review]
changes for a 32-bit build

Here's a patch that gets mesa+swr building on 32-bit and a smoketest of glxgears passes.  We haven't tried this configuration before, so there might be problems lurking.

Fixing this uncovered some typedef cruft which I thought we'd removed from the tree; will probably fix that in other patches.

Change committed.

Commit: 90f9df3210b5b66585007ec4836bfca498fd45f0

Thanks for looking into it, I am still getting a build failure, however:
https://launchpadlibrarian.net/247150700/buildlog_ubuntu-wily-i386.mesa_11.3~git1603081656.90f9df~gd~w_BUILDING.txt.gz

  CCLD     libllvmpipe.la
ar: `u' modifier ignored since `D' is the default (see `U')
make[4]: Leaving directory '/«PKGBUILDDIR»/build/dri/src/gallium/drivers/llvmpipe'
Making all in drivers/swr
make[4]: Entering directory '/«PKGBUILDDIR»/build/dri/src/gallium/drivers/swr'
  CXX      swr_loader.lo
  CXXLD    libmesaswr.la
ar: `u' modifier ignored since `D' is the default (see `U')
make[4]: Leaving directory '/«PKGBUILDDIR»/build/dri/src/gallium/drivers/swr'
Making all in drivers/swr/avx
make[4]: Entering directory '/«PKGBUILDDIR»/build/dri/src/gallium/drivers/swr/avx'
python2.7  \
	../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/scripts/gen_knobs.py \
	rasterizer/scripts
python2.7  \
	../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/jitter/scripts/gen_llvm_types.py \
	--input ../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/core/state.h \
	--output rasterizer/jitter/state_llvm.h
python2.7  \
	../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/jitter/scripts/gen_llvm_ir_macros.py \
	--input /usr/lib/llvm-3.8/include/llvm/IR/IRBuilder.h \
	--output rasterizer/jitter/builder_gen.h \
	--gen_h
python2.7  \
	../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/jitter/scripts/gen_llvm_ir_macros.py \
	--input /usr/lib/llvm-3.8/include/llvm/IR/IRBuilder.h \
	--output rasterizer/jitter/builder_gen.cpp \
	--gen_cpp
python2.7  \
	../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/jitter/scripts/gen_llvm_ir_macros.py \
	--output rasterizer/jitter/builder_x86.h \
	--gen_x86_h
python2.7  \
	../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/jitter/scripts/gen_llvm_ir_macros.py \
	--output rasterizer/jitter/builder_x86.cpp \
	--gen_x86_cpp
/usr/bin/make  all-am
make[5]: Entering directory '/«PKGBUILDDIR»/build/dri/src/gallium/drivers/swr/avx'
  CXX      swr_clear.lo
In file included from ../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/core/state.h:31:0,
                 from ../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/jitter/fetch_jit.h:33,
                 from ../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/jitter/jit_api.h:33,
                 from ../../../../../../../src/gallium/drivers/swr/avx/../swr_context.h:30,
                 from ../../../../../../../src/gallium/drivers/swr/avx/../swr_clear.cpp:24:
../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/common/simdintrin.h: In function '__m256i _simdemu_mul_epi32(__m256i, __m256i)':
../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/common/simdintrin.h:101:34: warning: AVX vector return without AVX enabled changes the ABI [-Wpsabi]
 __m256i func(__m256i a, __m256i b)\
                                  ^
../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/common/simdintrin.h:141:1: note: in expansion of macro 'SIMD_EMU_EPI'
 SIMD_EMU_EPI(_simdemu_mul_epi32, _mm_mul_epi32)
 ^
  CXX      swr_context.lo
In file included from ../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/core/state.h:31:0,
                 from ../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/jitter/fetch_jit.h:33,
                 from ../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/jitter/jit_api.h:33,
                 from ../../../../../../../src/gallium/drivers/swr/avx/../swr_context.h:30,
                 from ../../../../../../../src/gallium/drivers/swr/avx/../swr_context.cpp:33:
../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/common/simdintrin.h: In function '__m256i _simdemu_mul_epi32(__m256i, __m256i)':
../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/common/simdintrin.h:101:34: warning: AVX vector return without AVX enabled changes the ABI [-Wpsabi]
 __m256i func(__m256i a, __m256i b)\
                                  ^
../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/common/simdintrin.h:141:1: note: in expansion of macro 'SIMD_EMU_EPI'
 SIMD_EMU_EPI(_simdemu_mul_epi32, _mm_mul_epi32)
 ^
  CXX      swr_draw.lo
In file included from ../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/core/api.h:37:0,
                 from ../../../../../../../src/gallium/drivers/swr/avx/../swr_screen.h:29,
                 from ../../../../../../../src/gallium/drivers/swr/avx/../swr_draw.cpp:24:
../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/common/simdintrin.h: In function '__m256i _simdemu_mul_epi32(__m256i, __m256i)':
../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/common/simdintrin.h:101:34: warning: AVX vector return without AVX enabled changes the ABI [-Wpsabi]
 __m256i func(__m256i a, __m256i b)\
                                  ^
../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/common/simdintrin.h:141:1: note: in expansion of macro 'SIMD_EMU_EPI'
 SIMD_EMU_EPI(_simdemu_mul_epi32, _mm_mul_epi32)
 ^
  CXX      swr_screen.lo
In file included from ../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/core/api.h:37:0,
                 from ../../../../../../../src/gallium/drivers/swr/avx/../swr_screen.h:29,
                 from ../../../../../../../src/gallium/drivers/swr/avx/../swr_screen.cpp:38:
../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/common/simdintrin.h: In function '__m256i _simdemu_mul_epi32(__m256i, __m256i)':
../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/common/simdintrin.h:101:34: warning: AVX vector return without AVX enabled changes the ABI [-Wpsabi]
 __m256i func(__m256i a, __m256i b)\
                                  ^
../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/common/simdintrin.h:141:1: note: in expansion of macro 'SIMD_EMU_EPI'
 SIMD_EMU_EPI(_simdemu_mul_epi32, _mm_mul_epi32)
 ^
  CXX      swr_state.lo
In file included from ../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/core/state.h:31:0,
                 from ../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/jitter/fetch_jit.h:33,
                 from ../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/jitter/jit_api.h:33,
                 from ../../../../../../../src/gallium/drivers/swr/avx/../swr_state.cpp:25:
../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/common/simdintrin.h: In function '__m256i _simdemu_mul_epi32(__m256i, __m256i)':
../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/common/simdintrin.h:101:34: warning: AVX vector return without AVX enabled changes the ABI [-Wpsabi]
 __m256i func(__m256i a, __m256i b)\
                                  ^
../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/common/simdintrin.h:141:1: note: in expansion of macro 'SIMD_EMU_EPI'
 SIMD_EMU_EPI(_simdemu_mul_epi32, _mm_mul_epi32)
 ^
In file included from /usr/lib/gcc/i686-linux-gnu/5/include/smmintrin.h:811:0,
                 from /usr/lib/gcc/i686-linux-gnu/5/include/x86intrin.h:41,
                 from ../../../../../../../src/gallium/drivers/swr/avx/../rasterizer/common/os.h:72,
                 from ../../../../../../../src/gallium/drivers/swr/avx/../swr_state.cpp:24:
/usr/lib/gcc/i686-linux-gnu/5/include/popcntintrin.h: In function 'void* swr_create_vs_state(pipe_context*, const pipe_shader_state*)':
/usr/lib/gcc/i686-linux-gnu/5/include/popcntintrin.h:35:1: error: inlining failed in call to always_inline 'int _mm_popcnt_u32(unsigned int)': target specific option mismatch
 _mm_popcnt_u32 (unsigned int __X)
 ^
../../../../../../../src/gallium/drivers/swr/avx/../swr_state.cpp:346:28: error: called from here
              _mm_popcnt_u32(swr_vs->soState.streamMasks[i]);
                            ^
Makefile:1013: recipe for target 'swr_state.lo' failed

Hmm, interesting problem.  In drivers/swr/avx[2], we set the compiler code generation in AM_CXXFLAGS with -march=<appropriate architecture>.  This works fine for normal development building of mesa.

In the distribution build environment, configure is passed a rather extensive CXXFLAGS, which contains in part "-march=pentium3 -mtune=generic".  Automake's philosophy is that the users' wishes should win out, so CXXFLAGS comes after AM_CXXFLAGS on the command line and the instruction set is restricted to pentium3.

Thanks for the nice check Tim.

Looks like a bug on our end - the configure check should honour the FLAGS, thus we'll warn/error and one won't be able to build swr with -march=pentium3.

Won't you run into the same problem with 64-bit builds in your system?  I'd imagine that they're probably configured with -march=core2 for the widest target platform, which will run into the same issue with swr's direct avx intrinsic usage.

(In reply to Emil Velikov from comment #5)
> Thanks for the nice check Tim.
> 
> Looks like a bug on our end - the configure check should honour the FLAGS,
> thus we'll warn/error and one won't be able to build swr with
> -march=pentium3.

Other way around - you want to ignore the user's request and build with AVX anyways. The code in question should only be loaded if AVX (or AVX2) are detected at runtime.

FWIW this is how we do it for some sse4 stuff:

src/mesa/Makefile.am:

libmesa_sse41_la_SOURCES = \
        main/streaming-load-memcpy.c \
        main/streaming-load-memcpy.h \
        main/sse_minmax.c \
        main/sse_minmax.h
libmesa_sse41_la_CFLAGS = $(AM_CFLAGS) $(SSE41_CFLAGS)

Presumably a similar approach should work for swr?

(In reply to Tim Rowley from comment #6)
> Won't you run into the same problem with 64-bit builds in your system?  I'd
> imagine that they're probably configured with -march=core2 for the widest
> target platform, which will run into the same issue with swr's direct avx
> intrinsic usage.

The arch tuning is only applied on 32 bit, the 64 bit already defaults to a more recent CPU.

In reply to Ilia Mirkin from comment #7)
> (In reply to Emil Velikov from comment #5)
> > Thanks for the nice check Tim.
> > 
> > Looks like a bug on our end - the configure check should honour the FLAGS,
> > thus we'll warn/error and one won't be able to build swr with
> > -march=pentium3.
> 
> Other way around - you want to ignore the user's request and build with AVX
> anyways. The code in question should only be loaded if AVX (or AVX2) are
> detected at runtime.

This make more sense than requiring disabling pentium3 optimization, as the rationale for arch tuning on 32 bit is to tune for a newer CPU (pentium3) rather than the default (i586 or something). Else we'll get AVX on swr and standard i586 on all other code.

(In reply to Ilia Mirkin from comment #7)

> Other way around - you want to ignore the user's request and build with AVX
> anyways. The code in question should only be loaded if AVX (or AVX2) are
> detected at runtime.

Eek, brain freeze moment. You're right.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/mesa/mesa/issues/185.