[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-30 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. Tests reverted in https://reviews.llvm.org/rL341118 Repository: rL LLVM https://reviews.llvm.org/D50845 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-30 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. Reverted in https://reviews.llvm.org/rL341115 Repository: rL LLVM https://reviews.llvm.org/D50845 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-30 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. That, and r340967 https://reviews.llvm.org/D51441. I'm running check-clang now and will land reverted changes shortly. Repository: rL LLVM https://reviews.llvm.org/D50845 ___ cfe-commits mailing list

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-30 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. In https://reviews.llvm.org/D50845#1219865, @gtbercea wrote: > In https://reviews.llvm.org/D50845#1219859, @Hahnfeld wrote: > > > removing `InitializePredefinedAuxMacros` and the new test completely should > > do. > > > Yep they also contain

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-30 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea added a comment. In https://reviews.llvm.org/D50845#1219859, @Hahnfeld wrote: > removing `InitializePredefinedAuxMacros` and the new test completely should > do. Yep they also contain https://reviews.llvm.org/D51312 in case you're rolling back individual commits. Repository: rL

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-30 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. In https://reviews.llvm.org/D50845#1219853, @tra wrote: > In https://reviews.llvm.org/D50845#1219819, @Hahnfeld wrote: > > > Ok, the top preprocessor condition for that function is `#ifndef > > __SSE2_MATH__` - the exact same macro that was part of the motivation. Can

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-30 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. In https://reviews.llvm.org/D50845#1219819, @Hahnfeld wrote: > Ok, the top preprocessor condition for that function is `#ifndef > __SSE2_MATH__` - the exact same macro that was part of the motivation. Can > you please test compiling a simple C file (including `math.h`)

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-30 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. In https://reviews.llvm.org/D50845#1219746, @tra wrote: > In our case the headers from a relatively old glibc and compiler errors out > on this: > > /* This function is used in the `isfinite' macro. */ > __MATH_INLINE int > __NTH (__finite (double __x)) > { >

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-30 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. In https://reviews.llvm.org/D50845#1219797, @tra wrote: > I've sent out https://reviews.llvm.org/D51501. It unbreaks CUDA compilation > and keeps OpenMP unchanged. I think a full revert would make more sense. And you definitely want to reinstantiate // FIXME:

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-30 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. I've sent out https://reviews.llvm.org/D51501. It unbreaks CUDA compilation and keeps OpenMP unchanged. Repository: rL LLVM https://reviews.llvm.org/D50845 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-30 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea added a comment. In https://reviews.llvm.org/D50845#1219746, @tra wrote: > Also, whatever macros we generate do not prevent headers from using x86 > inline assembly. I see quite a few inline asm code in preprocessed output. > The headers are from libc ~2.19. Did you try adding

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-30 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. In https://reviews.llvm.org/D50845#1219733, @Hahnfeld wrote: > In https://reviews.llvm.org/D50845#1219726, @gtbercea wrote: > > > In general, it looks like this patch leads to some host macros having to be > > defined again for the auxiliary triple case. It is not clear to

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-30 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. In our case the headers from a relatively old glibc and compiler errors out on this: /* This function is used in the `isfinite' macro. */ __MATH_INLINE int __NTH (__finite (double __x)) { return (__extension__ (union { double __d; int __i[2]; })

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-30 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. In https://reviews.llvm.org/D50845#1219726, @gtbercea wrote: > In general, it looks like this patch leads to some host macros having to be > defined again for the auxiliary triple case. It is not clear to me how to > exhaustively identify the missing macros, so far

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-30 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea added a comment. In https://reviews.llvm.org/D50845#1219709, @tra wrote: > FYI. This breaks our CUDA compilation. I haven't figured out what exactly is > wrong yet. I may need to unroll the patch if the fix is not obvious. Agreed. Patches https://reviews.llvm.org/D51446 and

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-30 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. Do you have invocations or headers that don't work? The problem is that the previous code defined all macros unconditionally, so it will afterwards be hard to find the necessary macros... Repository: rL LLVM https://reviews.llvm.org/D50845

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-30 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In https://reviews.llvm.org/D50845#1219709, @tra wrote: > FYI. This breaks our CUDA compilation. I haven't figured out what exactly is > wrong yet. I may need to unroll the patch if the fix is not obvious. +1. I think this patch must be reverted, it breaks compilation

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-30 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. FYI. This breaks our CUDA compilation. I haven't figured out what exactly is wrong yet. I may need to unroll the patch if the fix is not obvious. Repository: rL LLVM https://reviews.llvm.org/D50845 ___ cfe-commits mailing

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-25 Thread Jonas Hahnfeld via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL340681: [CUDA/OpenMP] Define only some host macros during device compilation (authored by Hahnfeld, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-25 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. In https://reviews.llvm.org/D50845#1212643, @tra wrote: > Please keep an eye on CUDA buildbot > http://lab.llvm.org:8011/builders/clang-cuda-build. > It runs fair amount of tests with libc++ and handful of libstdc++ versions > and may a canary if these changes break

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-25 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld updated this revision to Diff 162543. Hahnfeld added a comment. Based on libc++ I guessed some more macros that may be needed on macOS and Windows. As I can't test myself if somebody else could report if this change is regressing CUDA support on these platforms.

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-24 Thread Artem Belevich via Phabricator via cfe-commits
tra accepted this revision. tra added a comment. This revision is now accepted and ready to land. Please keep an eye on CUDA buildbot http://lab.llvm.org:8011/builders/clang-cuda-build. It runs fair amount of tests with libc++ and handful of libstdc++ versions and may a canary if these changes

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-24 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld updated this revision to Diff 162328. Hahnfeld added a comment. Herald added a subscriber: krytarowski. Add required macros for compiling C++ code. https://reviews.llvm.org/D50845 Files: lib/Frontend/InitPreprocessor.cpp test/Preprocessor/aux-triple.c test/SemaCUDA/builtins.cu

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-23 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. In https://reviews.llvm.org/D50845#1211463, @gregrodgers wrote: > What am I missing? As discussed above this patch doesn't fix this problem. However we need `__x86_64__` because `bits/wordsize.h` will use it to determine if we are 64- or 32-bit. Repository: rC

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-23 Thread Greg Rodgers via Phabricator via cfe-commits
gregrodgers added a comment. I have a longer comment on header files, but let me first understand this patch. IIUC,the concept of this patch is to fake the macros to think it is seeing a host on the device patch. if ((LangOpts.CUDA || LangOpts.OpenMPIsDevice) && PP.getAuxTargetInfo())

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-23 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. The discussion kind of moved away from the original patch, probably because the problem is larger than the defition of some host macros. However I still think that this patch improves the situation. Repository: rC Clang https://reviews.llvm.org/D50845

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-17 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. In https://reviews.llvm.org/D50845#1204340, @ABataev wrote: > In https://reviews.llvm.org/D50845#1204216, @Hahnfeld wrote: > > > Got that, I agree on the conservative approach: If we find a function to be > > called that wasn't checked (because it wasn't implicitly

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-17 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In https://reviews.llvm.org/D50845#1204216, @Hahnfeld wrote: > In https://reviews.llvm.org/D50845#1204210, @ABataev wrote: > > > > Right, warning wasn't a good thought. We really want strict checking and > > > would have to error out when we find a function that wasn't

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-17 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. In https://reviews.llvm.org/D50845#1202540, @ABataev wrote: > Maybe for device compilation we also should define `__NO_MATH_INLINES` and > `__NO_STRING_INLINES` macros to disable inline assembly in glibc? Coming back to this original question: - I just searched the

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-17 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. In https://reviews.llvm.org/D50845#1204210, @ABataev wrote: > > Right, warning wasn't a good thought. We really want strict checking and > > would have to error out when we find a function that wasn't implicitly > > `declare target` on the host. > > I meant to ask

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-17 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. > Right, warning wasn't a good thought. We really want strict checking and > would have to error out when we find a function that wasn't implicitly > `declare target` on the host. > I meant to ask how common that would be? If that's only some known functions > we

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-17 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. In https://reviews.llvm.org/D50845#1203991, @ABataev wrote: > In https://reviews.llvm.org/D50845#1203973, @Hahnfeld wrote: > > > In https://reviews.llvm.org/D50845#1203967, @ABataev wrote: > > > > > I thought about this approach already. But it won't work in general.

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-17 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In https://reviews.llvm.org/D50845#1203973, @Hahnfeld wrote: > In https://reviews.llvm.org/D50845#1203967, @ABataev wrote: > > > In https://reviews.llvm.org/D50845#1203961, @Hahnfeld wrote: > > > > > In https://reviews.llvm.org/D50845#1202973, @ABataev wrote: > > > > > >

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-17 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. In https://reviews.llvm.org/D50845#1203967, @ABataev wrote: > In https://reviews.llvm.org/D50845#1203961, @Hahnfeld wrote: > > > In https://reviews.llvm.org/D50845#1202973, @ABataev wrote: > > > > > > So ideally I think Clang should determine which functions are really

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-17 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In https://reviews.llvm.org/D50845#1203961, @Hahnfeld wrote: > In https://reviews.llvm.org/D50845#1202973, @ABataev wrote: > > > > So ideally I think Clang should determine which functions are really > > > `declare target` (either explicit or implicit) and only run

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-17 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. In https://reviews.llvm.org/D50845#1202973, @ABataev wrote: > > So ideally I think Clang should determine which functions are really > > `declare target` (either explicit or implicit) and only run semantical > > analysis on them. If a function is then found to be

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-16 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. In https://reviews.llvm.org/D50845#1203031, @gtbercea wrote: > In https://reviews.llvm.org/D50845#1202991, @hfinkel wrote: > > > In https://reviews.llvm.org/D50845#1202965, @Hahnfeld wrote: > > > > > In https://reviews.llvm.org/D50845#1202963, @hfinkel wrote: > > > > > > >

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-16 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea added a comment. In https://reviews.llvm.org/D50845#1202991, @hfinkel wrote: > In https://reviews.llvm.org/D50845#1202965, @Hahnfeld wrote: > > > In https://reviews.llvm.org/D50845#1202963, @hfinkel wrote: > > > > > As a result, we should really have a separate header that has those >

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-16 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea added a comment. In https://reviews.llvm.org/D50845#1202973, @ABataev wrote: > >> If I understand it correctly, the root cause of this exercise is that we > >> want to compile for GPU using plain C. CUDA avoids this issue by > >> separating device and host code via target attributes

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-16 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D50845#1202965, @Hahnfeld wrote: > In https://reviews.llvm.org/D50845#1202963, @hfinkel wrote: > > > As a result, we should really have a separate header that has those > > actually-available functions. When targeting NVPTX, why don't we have

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-16 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. >> If I understand it correctly, the root cause of this exercise is that we >> want to compile for GPU using plain C. CUDA avoids this issue by separating >> device and host code via target attributes and clang has few special cases >> to ignore inline assembly errors

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-16 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. In https://reviews.llvm.org/D50845#1202963, @hfinkel wrote: > As a result, we should really have a separate header that has those > actually-available functions. When targeting NVPTX, why don't we have the > included math.h be CUDA's math.h? In the end, those are the

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-16 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. > Another option would be to implement some sort of attribute-based > overloading. Then OpenMP can provide its own version of the device-side > library function without clashing with system headers. I'm thinking about what the desired behavior is here. So, if we have a

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-16 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. In https://reviews.llvm.org/D50845#1202838, @tra wrote: > In https://reviews.llvm.org/D50845#1202551, @ABataev wrote: > > > In https://reviews.llvm.org/D50845#1202550, @Hahnfeld wrote: > > > > > In https://reviews.llvm.org/D50845#1202540, @ABataev wrote: > > > > > > >

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-16 Thread Artem Belevich via Phabricator via cfe-commits
tra added a subscriber: pcc. tra added a comment. In https://reviews.llvm.org/D50845#1202551, @ABataev wrote: > In https://reviews.llvm.org/D50845#1202550, @Hahnfeld wrote: > > > In https://reviews.llvm.org/D50845#1202540, @ABataev wrote: > > > > > Maybe for device compilation we also should

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-16 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In https://reviews.llvm.org/D50845#1202550, @Hahnfeld wrote: > In https://reviews.llvm.org/D50845#1202540, @ABataev wrote: > > > Maybe for device compilation we also should define `__NO_MATH_INLINES` and > > `__NO_STRING_INLINES` macros to disable inline assembly in

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-16 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. In https://reviews.llvm.org/D50845#1202540, @ABataev wrote: > Maybe for device compilation we also should define `__NO_MATH_INLINES` and > `__NO_STRING_INLINES` macros to disable inline assembly in glibc? The problem is that `__NO_MATH_INLINES` doesn't even avoid all

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-16 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. Maybe for device compilation we also should define `__NO_MATH_INLINES` and `__NO_STRING_INLINES` macros to disable inline assembly in glibc? Repository: rC Clang https://reviews.llvm.org/D50845 ___ cfe-commits mailing

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-16 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added inline comments. Comment at: test/SemaCUDA/builtins.cu:15-17 +#if !defined(__x86_64__) +#error "Expected to see preprocessor macros from the host." #endif @tra I'm not sure here: Do we want `__PTX__` to be defined during host compilation? I

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

2018-08-16 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld created this revision. Hahnfeld added reviewers: tra, gtbercea, hfinkel. Herald added subscribers: cfe-commits, guansong. When compiling CUDA or OpenMP device code Clang parses header files that expect certain predefined macros from the host architecture. To make this work the compiler