[PATCH] D91088: [CUDA][HIP] Fix capturing reference to host variable

This revision was automatically updated to reflect the committed changes. yaxunl marked an inline comment as done. Closed by commit rGcd95338ee302: [CUDA][HIP] Fix capturing reference to host variable (authored by yaxunl). Herald added a project: clang. Changed prior to commit: https://reviews.

[PATCH] D91088: [CUDA][HIP] Fix capturing reference to host variable

yaxunl marked 3 inline comments as done. yaxunl added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:1957 +// host variable in a device or host device lambda. +static bool isCapturingReferenceToHostVarInCUDADeviceLambda(Sema &S, +

[PATCH] D91088: [CUDA][HIP] Fix capturing reference to host variable

tra accepted this revision. tra added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Sema/SemaExpr.cpp:1957 +// host variable in a device or host device lambda. +static bool isCapturingReferenceToHostVarInCUDADeviceLambda(Sema &S, +

[PATCH] D91088: [CUDA][HIP] Fix capturing reference to host variable

yaxunl marked 2 inline comments as done. yaxunl added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:1944 + // host variable in a device or host device lambda. + auto IsCapturingReferenceToHostVarInCUDADeviceLambda = [&](VarDecl *VD) { +if (!getLangOpts().CUDA ||

[PATCH] D91088: [CUDA][HIP] Fix capturing reference to host variable

yaxunl updated this revision to Diff 308519. yaxunl marked an inline comment as done. yaxunl added a comment. extract lambda as a function CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91088/new/ https://reviews.llvm.org/D91088 Files: clang/lib/CodeGen/CGExpr.cpp clang/lib/Sema/Sem

[PATCH] D91088: [CUDA][HIP] Fix capturing reference to host variable

tra added a comment. LGTM in general. Comment at: clang/lib/Sema/SemaExpr.cpp:1944 + // host variable in a device or host device lambda. + auto IsCapturingReferenceToHostVarInCUDADeviceLambda = [&](VarDecl *VD) { +if (!getLangOpts().CUDA || !VD->hasInit()) ---

[PATCH] D91088: [CUDA][HIP] Fix capturing reference to host variable

yaxunl marked 2 inline comments as done. yaxunl added a comment. ping Comment at: clang/lib/Sema/SemaExpr.cpp:357 + if (LangOpts.CUDAIsDevice) { +auto *FD = dyn_cast_or_null(CurContext); tra wrote: > This could use a comment about why we only check `D->H

[PATCH] D91088: [CUDA][HIP] Fix capturing reference to host variable

yaxunl updated this revision to Diff 304566. yaxunl edited the summary of this revision. yaxunl added a comment. Separate diagnose of host variable to another patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91088/new/ https://reviews.llvm.org/D91088 Files: clang/lib/CodeGen/CGEx

[PATCH] D91088: [CUDA][HIP] Fix capturing reference to host variable

tra added a comment. In D91088#2388496 , @yaxunl wrote: > added diagnosing referencing host variable in device functions Thank you for fixing this. The new changes look good, but I think they should be in a separate patch. Comment at:

[PATCH] D91088: [CUDA][HIP] Fix capturing reference to host variable

yaxunl updated this revision to Diff 304494. yaxunl edited the summary of this revision. yaxunl added a comment. added diagnosing referencing host variable in device functions CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91088/new/ https://reviews.llvm.org/D91088 Files: clang/includ

[PATCH] D91088: [CUDA][HIP] Fix capturing reference to host variable

tra added a comment. Ouch. This is definitely a bug. https://godbolt.org/z/faPWcv Granted, the compilation will eventually fail in most cases due to ptxas failing to resolve the reference to the variable we do not generate on the device, but it's something we should catch in Sema. Filed https

[PATCH] D91088: [CUDA][HIP] Fix capturing reference to host variable

yaxunl added inline comments. Comment at: clang/test/CodeGenCUDA/lambda-reference-var.cu:61 + [=](){ +int &ref = global_device_var; +ref++; tra wrote: > Do we have current Sema tests that verify that we we would not allow > accessing host vars here? cur

[PATCH] D91088: [CUDA][HIP] Fix capturing reference to host variable

tra added a subscriber: rsmith. tra added a comment. The use case in the tests makes sense, but I'd like to have a second opinion on whether it may have unintended consequences. Lambdas tend to bring interesting corner cases. @rsmith Richard, do you see any issues with this approach? ===

[PATCH] D91088: [CUDA][HIP] Fix capturing reference to host variable

yaxunl updated this revision to Diff 303905. yaxunl added a comment. remove debug code CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91088/new/ https://reviews.llvm.org/D91088 Files: clang/lib/CodeGen/CGExpr.cpp clang/lib/Sema/SemaExpr.cpp clang/test/CodeGenCUDA/lambda-reference-

[PATCH] D91088: [CUDA][HIP] Fix capturing reference to host variable

yaxunl created this revision. yaxunl added reviewers: tra, rjmccall. yaxunl requested review of this revision. In C++ when a reference variable is captured by copy, the lambda is supposed to make a copy of the referenced variable in the captures and refer to the copy in the lambda. Therefore, it i