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
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
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
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
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
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
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`)
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))
> {
>
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:
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
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
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
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]; })
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
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
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
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
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
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:
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
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.
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
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
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
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())
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
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
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
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
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
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
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.
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:
> > >
> > >
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
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
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
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:
> > >
> > > >
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
>
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
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
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
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
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
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:
> > >
> > > >
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
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
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
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
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
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
50 matches
Mail list logo