[PATCH] D108493: [HIP] Warn capture this pointer in device lambda

[Artem Belevich via Phabricator via cfe-commits]

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