tra accepted this revision.
tra added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clang/lib/Sema/SemaCUDA.cpp:881-882
+ } else if (Capture.isThisCapture() && !LangOpts.HIP) {
+ // Capture of this pointer is allowed for HIP since this pointer may be
+ // pointing to managed memory which is accessible on both device and
+ // host sides.
----------------
tra wrote:
> yaxunl wrote:
> > tra wrote:
> > > I assume there's no easy way to tell if it's a managed pointer or not.
> > > Capturing a non-managed pointer would still be bad.
> > > Should we make it a warning instead?
> > >
> > >
> > warning makes sense. will do.
> >
> > Also this should apply to CUDA too. Although clang does not support
> > `__managed__` keyword for CUDA, `this` pointer may be allocated in managed
> > memory through host API's.
> > Also this should apply to CUDA too.
>
> I'm not sure I understand what you mean.
>
> Do you suggest that the warning should be issued for both HIP and CUDA? If
> so, the diag already applies to CUDA.
>
> Or that we should allow capturing wrong-side `this` w/o diagnostics for both
> CUDA and HIP?
>
> I'm fine with the former. But I do not think we should silently allow
> capturing wrong-side `this` for CUDA, because it will be an error most of the
> time.
Ah. Never mind. I've replied before your latest patch update.
LGTM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108493/new/
https://reviews.llvm.org/D108493
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
