search

PCSX2 / pcsx2

PCSX2 - The Playstation 2 Emulator
https://pcsx2.net
GNU General Public License v3.0
11.72k stars 1.62k forks source link

Build failure with clang-8.0.1 #3096

Closed orbea closed 3 years ago

orbea commented 5 years ago

PCSX2 version: https://github.com/PCSX2/pcsx2/commit/99f814d3764bc4ce87491ebeedac84762b3d68f3

Description of the issue: When building pcsx2 with clang the build will fail.

[ 57%] Building CXX object plugins/GSdx/CMakeFiles/GSdx.dir/PSX/GPU.cpp.o
cd /tmp/SBo/pcsx2/build/plugins/GSdx && /usr/local/bin/clang++ -m32  -DDISABLE_BUILD_DATE -DDOC_DIR_COMPILATION=/usr/doc/pcsx2-2019.08.26_99f814d -DGAMEINDEX_DIR_COMPILATION=/usr/share/games/PCSX2 -DGSdx_EXPORTS -DPLUGIN_DIR_COMPILATION=/usr/lib/games/PCSX2 -DWXUSINGDLL -DXDG_STD -D_ARCH_32=1 -D_FILE_OFFSET_BITS=64 -D_M_X86=1 -D_M_X86_32=1 -D__WXGTK__ -I/usr/include/SDL2 -I/usr/include/freetype2 -I/usr/include/glib-2.0 -I/usr/lib32/glib-2.0/include -I/usr/include/atk-1.0 -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/gtk-2.0 -I/usr/lib32/gtk-2.0/include -I/usr/include/libxml2 -I/usr/include/harfbuzz -I/tmp/SBo/pcsx2/common/include -I/tmp/SBo/pcsx2/common/include/Utilities -I/tmp/SBo/pcsx2/common/include/x86emitter -I/tmp/SBo/pcsx2/build/common/include -I/tmp/SBo/pcsx2/build/plugins/GSdx -I/tmp/SBo/pcsx2/plugins/GSdx/. -isystem /usr/lib/wx/include/gtk2-unicode-3.0 -isystem /usr/include/wx-3.0  -O2 -msse -msse2 -mfxsr -mxsave -march=i686 -pipe -fvisibility=hidden -pthread -fno-builtin-strcmp -fno-builtin-memcmp -mfpmath=sse -Wall -Wextra -Wno-attributes -Wno-unused-function -Wno-unused-parameter -Wno-missing-field-initializers -Wno-overloaded-virtual -Wno-unused-value   -Wno-deprecated-register -Wno-c++14-extensions -Wstrict-aliasing -Wstrict-overflow=1  -D_FORTIFY_SOURCE=2  -Wformat -Wformat-security -DNDEBUG  -O2    -std=c++11 -Wno-invalid-offsetof -pthread   -fno-operator-names -Wno-unknown-pragmas -Wno-parentheses -Wunused-variable  -fno-operator-names -Wno-unknown-pragmas -Wno-parentheses -Wunused-variable -DXDG_STD -o CMakeFiles/GSdx.dir/PSX/GPU.cpp.o -c /tmp/SBo/pcsx2/plugins/GSdx/PSX/GPU.cpp
In file included from /tmp/SBo/pcsx2/plugins/GSdx/PSX/GPU.cpp:23:
In file included from /tmp/SBo/pcsx2/plugins/GSdx/./GSdx.h:25:
In file included from /tmp/SBo/pcsx2/plugins/GSdx/./GS.h:40:
In file included from /tmp/SBo/pcsx2/plugins/GSdx/./GSVector.h:100:
/tmp/SBo/pcsx2/plugins/GSdx/./GSVector4i.h:769:21: error: argument value -4 is outside the valid range [0, 255] [-Wargument-outside-range]
                return GSVector4i(_mm_srli_si128(m, i));
                                  ^~~~~~~~~~~~~~~~~~~~
/usr/bin/../lib64/clang/8.0.1/include/emmintrin.h:3057:12: note: expanded from macro '_mm_srli_si128'
  (__m128i)__builtin_ia32_psrldqi128_byteshift((__v2di)(__m128i)(a), (int)(imm))
           ^                                                         ~~~~~~~~~~
/tmp/SBo/pcsx2/plugins/GSdx/./GSVector4i.h:783:28: note: in instantiation of function template specialization 'GSVector4i::srl<-4>' requested here
                else if(i < 32) return v.srl<i - 16>();
                                         ^
/tmp/SBo/pcsx2/plugins/GSdx/./GSBlock.h:1402:12: note: in instantiation of function template specialization 'GSVector4i::srl<12>' requested here
                        v4 = v4.srl<12>(v5);
                                ^
In file included from /tmp/SBo/pcsx2/plugins/GSdx/PSX/GPU.cpp:23:
In file included from /tmp/SBo/pcsx2/plugins/GSdx/./GSdx.h:25:
In file included from /tmp/SBo/pcsx2/plugins/GSdx/./GS.h:40:
In file included from /tmp/SBo/pcsx2/plugins/GSdx/./GSVector.h:100:
/tmp/SBo/pcsx2/plugins/GSdx/./GSVector4i.h:769:21: error: argument value -8 is outside the valid range [0, 255] [-Wargument-outside-range]
                return GSVector4i(_mm_srli_si128(m, i));
                                  ^~~~~~~~~~~~~~~~~~~~
/usr/bin/../lib64/clang/8.0.1/include/emmintrin.h:3057:12: note: expanded from macro '_mm_srli_si128'
  (__m128i)__builtin_ia32_psrldqi128_byteshift((__v2di)(__m128i)(a), (int)(imm))
           ^                                                         ~~~~~~~~~~
/tmp/SBo/pcsx2/plugins/GSdx/./GSVector4i.h:783:28: note: in instantiation of function template specialization 'GSVector4i::srl<-8>' requested here
                else if(i < 32) return v.srl<i - 16>();
                                         ^
/tmp/SBo/pcsx2/plugins/GSdx/./GSBlock.h:1404:12: note: in instantiation of function template specialization 'GSVector4i::srl<8>' requested here
                        v4 = v5.srl<8>(v6);
                                ^
2 errors generated.
make[2]: *** [plugins/GSdx/CMakeFiles/GSdx.dir/build.make:91: plugins/GSdx/CMakeFiles/GSdx.dir/PSX/GPU.cpp.o] Error 1
make[2]: Leaving directory '/tmp/SBo/pcsx2/build'
make[1]: *** [CMakeFiles/Makefile2:678: plugins/GSdx/CMakeFiles/GSdx.dir/all] Error 2
make[1]: Leaving directory '/tmp/SBo/pcsx2/build'
make: *** [Makefile:130: all] Error 2

Full build log: pcsx2.txt

How to reproduce the issue:

export CC=clang
export CXX=clang++
# build pcsx2

Last known version to work: Unknown, I have not tested pcsx2 with clang before.

PC specifications: OS: Slackware64-current clang: 8.0.1

orbea commented 5 years ago

Could this be because its finding /usr/bin/../lib64/clang/8.0.1/include/emmintrin.h instead of /usr/bin/../lib/clang/8.0.1/include/emmintrin.h? I have both.

Edit: The files are identical so its probably not the reason.

rcaridade145 commented 5 years ago

@orbea

error: argument value -4 is outside the valid range [0, 255]

From what it seems it only accepts positive numbers. Is -Wargument-outside-range also set on gcc? I'm not sure how gcc handles that. Found this tough:

https://github.com/jratcliff63367/sse2neon/blob/master/SSE2NEON.h#L961

orbea commented 5 years ago

gcc does not accept -Wargument-outside-range and I do not see any similar flags in their documentation.

http://tigcc.ticalc.org/doc/comopts.html#SEC8

rcaridade145 commented 5 years ago

@orbea http://clang-developers.42468.n3.nabble.com/Was-the-SSE2-mm-slli-si128-mm-srli-si128-behavior-change-in-r272246-intentional-td4052253.html .

orbea commented 5 years ago

Good find!

I foolishly thought that sending everything about 15 to the true leg of this ternary operator would prevent needing a mask on each of the immediate uses in the shufflevector. I'll fix it right away. AVX2 and AVX512 are similarly broken.

I wonder if there was ever a fix?

rcaridade145 commented 5 years ago

` case X86::BIbuiltin_ia32_psrldqi128_byteshift: case X86::BI__builtin_ia32_psrldqi256_byteshift: case X86::BIbuiltin_ia32_psrldqi512_byteshift: { unsigned ShiftVal = cast(Ops[1])->getZExtValue() & 0xff; llvm::Type ResultType = Ops[0]->getType(); // Builtin type is vXi64 so multiply by 8 to get bytes. unsigned NumElts = ResultType->getVectorNumElements() 8;

// If psrldq is shifting the vector more than 15 bytes, emit zero.
if (ShiftVal >= 16)
  return llvm::Constant::getNullValue(ResultType);`

from https://www.google.com/url?sa=t&rct=j&q=&esrc=s&source=web&cd=10&ved=2ahUKEwjfx4Pv5aXkAhUG1eAKHZjwBp8QFjAJegQIBBAB&url=http%3A%2F%2Fftp.tku.edu.tw%2FNetBSD%2FNetBSD-current%2Fsrc%2Fexternal%2Fbsd%2Fllvm%2Fdist%2Fclang%2Flib%2FCodeGen%2FCGBuiltin.cpp&usg=AOvVaw3Mg0e8Z5Nb3zInYkIcRhqZ after checking https://clang.llvm.org/doxygen/emmintrin_8h_source.html on line 3037 i'd say it is fixed. Now the question is should it be negative in the first place.

orbea commented 5 years ago

@lightningterror Does the clang build work on windows?

lightningterror commented 5 years ago

Actually I dunno, I just assumed the issue was on Linux since nobody said anything about windows.

orbea commented 5 years ago

I think clang works on windows too, might be worth checking?

orbea commented 5 years ago

With clang-9.0.0 it fails differently.

samu: job failed: /usr/local/bin/clang++ -m32  -DDISABLE_BUILD_DATE -DDOC_DIR_COMPILATION=/usr/doc/pcsx2-2019.09.29_45687cd -DGAMEINDEX_DIR_COMPILATION=/usr/share/games/PCSX2 -DPLUGIN_DIR_COMPILATION=/usr/lib/games/PCSX2 -DUSBnull_0_7_0_EXPORTS -DWXUSINGDLL -D_ARCH_32=1 -D_FILE_OFFSET_BITS=64 -D_M_X86=1 -D_M_X86_32=1 -D__WXGTK__ -I/usr/include/SDL2 -I/usr/include/freetype2 -I/usr/include/glib-2.0 -I/usr/lib32/glib-2.0/include -I/usr/include/atk-1.0 -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/gtk-2.0 -I/usr/lib32/gtk-2.0/include -I/usr/include/libxml2 -I/usr/include/harfbuzz -I../common/include -I../common/include/Utilities -I../common/include/x86emitter -Icommon/include -I../plugins/USBnull/. -isystem /usr/lib/wx/include/gtk2-unicode-3.0 -isystem /usr/include/wx-3.0 -O2 -msse -msse2 -mfxsr -mxsave -march=i686 -pipe -fvisibility=hidden -pthread -fno-builtin-strcmp -fno-builtin-memcmp -mfpmath=sse -Wall -Wextra -Wno-attributes -Wno-unused-function -Wno-unused-parameter -Wno-missing-field-initializers -Wno-overloaded-virtual -Wno-unused-value   -Wno-deprecated-register -Wno-c++14-extensions -Wstrict-aliasing -Wstrict-overflow=1  -D_FORTIFY_SOURCE=2  -Wformat -Wformat-security -DNDEBUG  -O2    -std=c++11 -Wno-invalid-offsetof -pthread    -fvisibility=hidden -Wall -Wno-parentheses -MD -MT plugins/USBnull/CMakeFiles/USBnull-0.7.0.dir/USB.cpp.o -MF plugins/USBnull/CMakeFiles/USBnull-0.7.0.dir/USB.cpp.o.d -o plugins/USBnull/CMakeFiles/USBnull-0.7.0.dir/USB.cpp.o -c ../plugins/USBnull/USB.cpp
samu: subcommand failed
[1/789] Building CXX object plugins/USBnull/CMakeFiles/USBnull-0.7.0.dir/USB.cpp.o
In file included from ../plugins/USBnull/USB.cpp:20:
In file included from ../plugins/USBnull/./USB.h:22:
In file included from ../common/include/PS2Edefs.h:49:
In file included from ../common/include/Pcsx2Defs.h:36:
../common/include/x86emitter/x86_intrin.h:73:38: error: redefinition of '__rord' as different kind of symbol
[[maybe_unused]] static unsigned int _rotr(unsigned int x, int s)
                                     ^
/usr/bin/../lib64/clang/9.0.0/include/ia32intrin.h:357:20: note: expanded from macro '_rotr'
#define _rotr(a,b) __rord((a), (b))
                   ^
/usr/bin/../lib64/clang/9.0.0/include/ia32intrin.h:330:1: note: previous definition is here
__rord(unsigned int __X, int __C) {
^
In file included from ../plugins/USBnull/USB.cpp:20:
In file included from ../plugins/USBnull/./USB.h:22:
In file included from ../common/include/PS2Edefs.h:49:
In file included from ../common/include/Pcsx2Defs.h:36:
../common/include/x86emitter/x86_intrin.h:73:57: error: expected ')'
[[maybe_unused]] static unsigned int _rotr(unsigned int x, int s)
                                                        ^
../common/include/x86emitter/x86_intrin.h:73:38: note: to match this '('
[[maybe_unused]] static unsigned int _rotr(unsigned int x, int s)
                                     ^
/usr/bin/../lib64/clang/9.0.0/include/ia32intrin.h:357:27: note: expanded from macro '_rotr'
#define _rotr(a,b) __rord((a), (b))
                          ^
In file included from ../plugins/USBnull/USB.cpp:20:
In file included from ../plugins/USBnull/./USB.h:22:
In file included from ../common/include/PS2Edefs.h:49:
In file included from ../common/include/Pcsx2Defs.h:36:
../common/include/x86emitter/x86_intrin.h:73:38: error: expected expression
[[maybe_unused]] static unsigned int _rotr(unsigned int x, int s)
                                     ^
/usr/bin/../lib64/clang/9.0.0/include/ia32intrin.h:357:30: note: expanded from macro '_rotr'
#define _rotr(a,b) __rord((a), (b))
                             ^
In file included from ../plugins/USBnull/USB.cpp:20:
In file included from ../plugins/USBnull/./USB.h:22:
In file included from ../common/include/PS2Edefs.h:49:
In file included from ../common/include/Pcsx2Defs.h:36:
../common/include/x86emitter/x86_intrin.h:73:64: error: expected ')'
[[maybe_unused]] static unsigned int _rotr(unsigned int x, int s)
                                                               ^
../common/include/x86emitter/x86_intrin.h:73:38: note: to match this '('
[[maybe_unused]] static unsigned int _rotr(unsigned int x, int s)
                                     ^
/usr/bin/../lib64/clang/9.0.0/include/ia32intrin.h:357:32: note: expanded from macro '_rotr'
#define _rotr(a,b) __rord((a), (b))
                               ^
In file included from ../plugins/USBnull/USB.cpp:20:
In file included from ../plugins/USBnull/./USB.h:22:
In file included from ../common/include/PS2Edefs.h:49:
In file included from ../common/include/Pcsx2Defs.h:36:
../common/include/x86emitter/x86_intrin.h:73:38: error: expected expression
[[maybe_unused]] static unsigned int _rotr(unsigned int x, int s)
                                     ^
/usr/bin/../lib64/clang/9.0.0/include/ia32intrin.h:357:35: note: expanded from macro '_rotr'
#define _rotr(a,b) __rord((a), (b))
                                  ^
In file included from ../plugins/USBnull/USB.cpp:20:
In file included from ../plugins/USBnull/./USB.h:22:
In file included from ../common/include/PS2Edefs.h:49:
In file included from ../common/include/Pcsx2Defs.h:36:
../common/include/x86emitter/x86_intrin.h:73:66: error: expected ';' after top level declarator
[[maybe_unused]] static unsigned int _rotr(unsigned int x, int s)
                                                                 ^
                                                                 ;
6 errors generated.
orbea commented 5 years ago

With the above referenced PR I am back to where I am started with clang-9.0.0.

[189/717] Building CXX object plugins/GSdx/CMakeFiles/GSdx.dir/Window/GSWnd.cpp.o
[190/717] Building CXX object plugins/GSdx/CMakeFiles/GSdx.dir/Window/GSSetting.cpp.o
[191/717] Building CXX object plugins/GSdx/CMakeFiles/GSdx.dir/Renderers/OpenCL/GSRendererCL.cpp.o
In file included from ../plugins/GSdx/Renderers/OpenCL/GSRendererCL.cpp:23:
In file included from ../plugins/GSdx/Renderers/OpenCL/GSRendererCL.h:24:
In file included from ../plugins/GSdx/./Renderers/Common/GSRenderer.h:24:
In file included from ../plugins/GSdx/./GSdx.h:25:
In file included from ../plugins/GSdx/./GS.h:40:
In file included from ../plugins/GSdx/./GSVector.h:100:
../plugins/GSdx/./GSVector4i.h:769:21: error: argument value -4 is outside the valid range [0, 255] [-Wargument-outside-range]
                return GSVector4i(_mm_srli_si128(m, i));
                                  ^~~~~~~~~~~~~~~~~~~~
/usr/bin/../lib64/clang/9.0.0/include/emmintrin.h:3038:12: note: expanded from macro '_mm_srli_si128'
  (__m128i)__builtin_ia32_psrldqi128_byteshift((__v2di)(__m128i)(a), (int)(imm))
           ^                samu: subcommand failed
                                         ~~~~~~~~~~
../plugins/GSdx/./GSVector4i.h:783:28: note: in instantiation of function template specialization 'GSVector4i::srl<-4>' requested here
                else if(i < 32) return v.srl<i - 16>();
                                         ^
../plugins/GSdx/./GSBlock.h:1402:12: note: in instantiation of function template specialization 'GSVector4i::srl<12>' requested here
                        v4 = v4.srl<12>(v5);
                                ^
In file included from ../plugins/GSdx/Renderers/OpenCL/GSRendererCL.cpp:23:
In file included from ../plugins/GSdx/Renderers/OpenCL/GSRendererCL.h:24:
In file included from ../plugins/GSdx/./Renderers/Common/GSRenderer.h:24:
In file included from ../plugins/GSdx/./GSdx.h:25:
In file included from ../plugins/GSdx/./GS.h:40:
In file included from ../plugins/GSdx/./GSVector.h:100:
../plugins/GSdx/./GSVector4i.h:769:21: error: argument value -8 is outside the valid range [0, 255] [-Wargument-outside-range]
                return GSVector4i(_mm_srli_si128(m, i));
                                  ^~~~~~~~~~~~~~~~~~~~
/usr/bin/../lib64/clang/9.0.0/include/emmintrin.h:3038:12: note: expanded from macro '_mm_srli_si128'
  (__m128i)__builtin_ia32_psrldqi128_byteshift((__v2di)(__m128i)(a), (int)(imm))
           ^                                                         ~~~~~~~~~~
../plugins/GSdx/./GSVector4i.h:783:28: note: in instantiation of function template specialization 'GSVector4i::srl<-8>' requested here
                else if(i < 32) return v.srl<i - 16>();
                                         ^
../plugins/GSdx/./GSBlock.h:1404:12: note: in instantiation of function template specialization 'GSVector4i::srl<8>' requested here
                        v4 = v5.srl<8>(v6);
                                ^
2 errors generated.
turtleli commented 5 years ago

I think this build error only occurs for SSE2 builds.

orbea commented 5 years ago

You are right, this only occurs with -DDISABLE_ADVANCE_SIMD=ON. I don't recall why I was using this option though.

For now I'll remove this in my builds and I now can build a working PCSX2 with clang.

orbea commented 4 years ago

This is also a problem with debug builds using clang.

orbea commented 4 years ago

This is enough to get the build to succeed, but I'm not sure what it does break if anything.

diff --git a/plugins/GSdx/GSBlock.h b/plugins/GSdx/GSBlock.h
index b73a0efc4..447048be1 100644
--- a/plugins/GSdx/GSBlock.h
+++ b/plugins/GSdx/GSBlock.h
@@ -1399,7 +1399,7 @@ public:
            v6 = GSVector4i::load<false>(src + srcpitch + 8);

            v0 = v4.upl32(v4.srl<3>()).upl64(v4.srl<6>().upl32(v4.srl<9>()));
-           v4 = v4.srl<12>(v5);
+           //v4 = v4.srl<12>(v5);
            v1 = v4.upl32(v4.srl<3>()).upl64(v4.srl<6>().upl32(v4.srl<9>()));
            v4 = v5.srl<8>(v6);
            v2 = v4.upl32(v4.srl<3>()).upl64(v4.srl<6>().upl32(v4.srl<9>()));
diff --git a/plugins/GSdx/GSVector4i.h b/plugins/GSdx/GSVector4i.h
index c68f90b64..a3dec8f1a 100644
--- a/plugins/GSdx/GSVector4i.h
+++ b/plugins/GSdx/GSVector4i.h
@@ -780,7 +780,7 @@ public:
        if(i == 0) return *this;
        else if(i < 16) return srl<i>() | v.sll<16 - i>();
        else if(i == 16) return v;
-       else if(i < 32) return v.srl<i - 16>();
+       //else if(i < 32) return v.srl<i - 16>();
        else return zero();

        #endif
orbea commented 4 years ago

Here is another workaround.

diff --git a/plugins/GSdx/GSBlock.h b/plugins/GSdx/GSBlock.h
index b73a0efc4..37965c6fe 100644
--- a/plugins/GSdx/GSBlock.h
+++ b/plugins/GSdx/GSBlock.h
@@ -1399,7 +1399,7 @@ public:
            v6 = GSVector4i::load<false>(src + srcpitch + 8);

            v0 = v4.upl32(v4.srl<3>()).upl64(v4.srl<6>().upl32(v4.srl<9>()));
-           v4 = v4.srl<12>(v5);
+           v4 = v4.srl<8>(v5);
            v1 = v4.upl32(v4.srl<3>()).upl64(v4.srl<6>().upl32(v4.srl<9>()));
            v4 = v5.srl<8>(v6);
            v2 = v4.upl32(v4.srl<3>()).upl64(v4.srl<6>().upl32(v4.srl<9>()));
diff --git a/plugins/GSdx/GSVector4i.h b/plugins/GSdx/GSVector4i.h
index c68f90b64..0c9f06e0f 100644
--- a/plugins/GSdx/GSVector4i.h
+++ b/plugins/GSdx/GSVector4i.h
@@ -780,7 +780,7 @@ public:
        if(i == 0) return *this;
        else if(i < 16) return srl<i>() | v.sll<16 - i>();
        else if(i == 16) return v;
-       else if(i < 32) return v.srl<i - 16>();
+       else if(i < 32) return v.srl<i - 8>();
        else return zero();

        #endif

I am not convinced this is an upstream bug rather than just bad code that gcc accepts and clang does not.