Page MenuHomePhabricator

[PowerPC] Fix x86 vector intrinsics wrapper compilation under C++
ClosedPublic

Authored by qiucf on May 30 2021, 10:41 PM.

Details

Diff Detail

Event Timeline

qiucf created this revision.May 30 2021, 10:41 PM
qiucf requested review of this revision.May 30 2021, 10:41 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 30 2021, 10:41 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
nemanjai accepted this revision.May 31 2021, 3:25 AM

LGTM. Thanks for fixing this.

clang/lib/Headers/ppc_wrappers/xmmintrin.h
31

I agree that it would be good to get rid of the extra space. :)

clang/test/CodeGen/ppc-xmmintrin.c
1433

I am not sure if this file has any tests that use the actual enumerators rather than an integer, but it seems like we should.

This revision is now accepted and ready to land.May 31 2021, 3:25 AM
This revision was landed with ongoing or failed builds.May 31 2021, 10:19 AM
This revision was automatically updated to reflect the committed changes.
bjope added inline comments.
clang/test/CodeGen/ppc-xmmintrin.c
10

Unfortunately I get some failures with this. Maybe because of an unstandard build setup.

We've started to use -DCLANG_DEFAULT_RTLIB=compiler-rt -DCLANG_DEFAULT_CXX_STDLIB=libc++ when building clang. And we also use -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON. Not sure if that is a setup that is asking for trouble. Anyway, when running this test case we end up with

: 'RUN: at line 10';   /workspace/llvm/build/bin/clang -x c++ -fsyntax-only -target powerpc64le-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS /workspace/clang/test/CodeGen/ppc-xmmintrin.c    -fno-discard-value-names -mllvm -disable-llvm-optzns
--
Exit Code: 1

Command Output (stderr):
--
In file included from /workspace/clang/test/CodeGen/ppc-xmmintrin.c:13:
In file included from /workspace/llvm/build/lib/clang/13.0.0/include/ppc_wrappers/xmmintrin.h:42:
In file included from /workspace/llvm/build/lib/clang/13.0.0/include/altivec.h:44:
In file included from /workspace/llvm/build/bin/../include/c++/v1/stddef.h:39:
/workspace/llvm/build/bin/../include/c++/v1/__config:13:10: fatal error: '__config_site' file not found
#include <__config_site>
         ^~~~~~~~~~~~~~~
1 error generated.

Not sure really how to solve that.

Maybe we should stop building like that?

Or there is a bug somewhere (such as that we only get the __config_site headers in target specific dirs for targets that we actually build runtimes for, maybe something that was missing in https://reviews.llvm.org/D97572)?
(Maybe @phosek could comment on that?)

Or this test case is missing some options to make it a bit more independent on the runtimes build setup?

qiucf added inline comments.Jun 2 2021, 9:51 PM
clang/test/CodeGen/ppc-xmmintrin.c
10

The tests relies on some system stuff. Is the failure related to this change? (or exposed by -x c++ option?)

bjope added inline comments.Jun 3 2021, 2:39 AM
clang/test/CodeGen/ppc-xmmintrin.c
10

Well, i guess it was exposed by -x c++ (in combination with D975729).

I don't really understand how things are supposed to work given the pre-target specific __config_site.

Since I only build libcxx for x86_64-unknown-linux-gnu, I only get a __config_site file for that specific triple inside bin/../include/x86_64-unknown-linux-gnu/c++/v1/__config_site in the build result. But a test case like this one, using a different triple, ends up including bin/../include/c++/v1/stddef.h, that wants wants to include <__config_site>, but it won't find any such include for the powerpc64le-unknown-linux-gnu triple.

qiucf added inline comments.Jun 7 2021, 1:50 AM
clang/test/CodeGen/ppc-xmmintrin.c
10

Hi, I tried a build with libcxx enabled on ppc64le, by default it still looks for stddef.h inside lib/clang/13.0.0/include. If I specify -isystem include/c++/v1, the error won't happen because __config_site is there.

I didn't try -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON since that makes build complaining bin/clang-tblgen: error while loading shared libraries: libc++.so.1: cannot open shared object file.

The revision is going to LLVM 12.0.1. And this failure doesn't look like a blocker, since (if I understand correctly) even without this patch, a simple test also triggers that in the condition you provided.