[PATCH] D78655: [HIP] Let lambda be host device by default

pfultz2 added a comment. > It captures addresses as seen at the point in time on the processor which > executed the constructor. Yea and the same happens when assigning the address to a pointer, which is later used on a different device. > Another point that capturing lambdas are not something

[PATCH] D78655: [HIP] Let lambda be host device by default

tra added a comment. In D78655#2013616 , @pfultz2 wrote: > > I.e. if I pass a mutable lambda by reference to the GPU kernel > > I dont think we are enabling passing host objects by reference through > functions. Although it could be possible to capture th

[PATCH] D78655: [HIP] Let lambda be host device by default

pfultz2 added a comment. > I.e. if I pass a mutable lambda by reference to the GPU kernel I dont think we are enabling passing host objects by reference through functions. Although it could be possible to capture the mutable lambda by reference by another lambda. > will the same lambda called

[PATCH] D78655: [HIP] Let lambda be host device by default

tra added a comment. In D78655#2013226 , @pfultz2 wrote: > > Not only the capture is an issue, like a regular function, lambda could > > also access non-local variables/functions. > > In practice this is not an issue. Hcc will implictly treat anything inl

[PATCH] D78655: [HIP] Let lambda be host device by default

pfultz2 added a comment. > Not only the capture is an issue, like a regular function, lambda could also > access non-local variables/functions. In practice this is not an issue. Hcc will implictly treat anything inlinable as host device, and user's are not confused or surprised when they use n

[PATCH] D78655: [HIP] Let lambda be host device by default

hliao added a comment. In D78655#2003837 , @yaxunl wrote: > In D78655#2003681 , @hliao wrote: > > > I though the goal of adding HD/D attributes for lambda is to make the > > static check easier as lambda used in de

[PATCH] D78655: [HIP] Let lambda be host device by default

hliao added a comment. In D78655#2011823 , @pfultz2 wrote: > > says we capture host variable reference in a device lambda. > > Is that required to be an error? I know @AlexVlx added support to hcc at one > point to capture host variables by reference. So

[PATCH] D78655: [HIP] Let lambda be host device by default

pfultz2 added a subscriber: AlexVlx. pfultz2 added a comment. > says we capture host variable reference in a device lambda. Is that required to be an error? I know @AlexVlx added support to hcc at one point to capture host variables by reference. So it seems to be possible for it to work correc

[PATCH] D78655: [HIP] Let lambda be host device by default

yaxunl marked an inline comment as done. yaxunl added a comment. In D78655#2003681 , @hliao wrote: > I though the goal of adding HD/D attributes for lambda is to make the static > check easier as lambda used in device code or device lambda is sensitive to

[PATCH] D78655: [HIP] Let lambda be host device by default

hliao added inline comments. Comment at: clang/test/CodeGenCUDA/lambda.cu:83 +void test_resolve(void) { + test_resolve_helper([&](){ overloaded();}); +} We are allowing regular lambda to be used in the device functions. That should be explicitly marked by makin

[PATCH] D78655: [HIP] Let lambda be host device by default

hliao requested changes to this revision. hliao added a comment. This revision now requires changes to proceed. In D78655#1997491 , @tra wrote: > Summoning @rsmith as I'm sure that there are interesting corner cases in > lambda handling that we didn't con

[PATCH] D78655: [HIP] Let lambda be host device by default

yaxunl marked 2 inline comments as done. yaxunl added inline comments. Comment at: clang/test/CodeGenCUDA/lambda.cu:16-26 +template +__global__ void g(F f) { f(); } + +template +void h(F f) { g<<<1,1>>>(f); } + +__device__ int a; tra wrote: > yaxunl wrote: > > tr

[PATCH] D78655: [HIP] Let lambda be host device by default

yaxunl updated this revision to Diff 260094. yaxunl added a comment. Added more tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78655/new/ https://reviews.llvm.org/D78655 Files: clang/lib/Sema/SemaCUDA.cpp clang/test/CodeGenCUDA/lambda.cu clang/test/SemaCUDA/lambda.cu Index:

[PATCH] D78655: [HIP] Let lambda be host device by default

tra added inline comments. Comment at: clang/test/CodeGenCUDA/lambda.cu:16-26 +template +__global__ void g(F f) { f(); } + +template +void h(F f) { g<<<1,1>>>(f); } + +__device__ int a; yaxunl wrote: > tra wrote: > > The test example may not be doing what it's se

[PATCH] D78655: [HIP] Let lambda be host device by default

yaxunl updated this revision to Diff 259957. yaxunl marked 2 inline comments as done. yaxunl added a comment. Fix typo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78655/new/ https://reviews.llvm.org/D78655 Files: clang/lib/Sema/SemaCUDA.cpp clang/test/CodeGenCUDA/lambda.cu clan

[PATCH] D78655: [HIP] Let lambda be host device by default

rjmccall added a comment. Seems reasonable Comment at: clang/test/CodeGenCUDA/lambda.cu:29 + +// Check lambda is emitted in device compilation and accessind device variable. +// DEV-LABEL: define internal void @_ZZ4mainENKUlvE_clEv Typo CHANGES SINCE LAST ACT

[PATCH] D78655: [HIP] Let lambda be host device by default

yaxunl marked 2 inline comments as done. yaxunl added inline comments. Comment at: clang/lib/Sema/SemaCUDA.cpp:723 +Method->addAttr(CUDADeviceAttr::CreateImplicit(Context)); +Method->addAttr(CUDAHostAttr::CreateImplicit(Context)); +return; pfultz2 wro

[PATCH] D78655: [HIP] Let lambda be host device by default

pfultz2 added inline comments. Comment at: clang/lib/Sema/SemaCUDA.cpp:723 +Method->addAttr(CUDADeviceAttr::CreateImplicit(Context)); +Method->addAttr(CUDAHostAttr::CreateImplicit(Context)); +return; Shouldn't we add these attributes if there are no h

[PATCH] D78655: [HIP] Let lambda be host device by default

yaxunl updated this revision to Diff 259452. yaxunl marked 2 inline comments as done. yaxunl added a comment. Add a negative test for lambda kernel. Add more checks to codegen test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78655/new/ https://reviews.llvm.org/D78655 Files: clang/

[PATCH] D78655: [HIP] Let lambda be host device by default

yaxunl updated this revision to Diff 259453. yaxunl added a comment. clean up test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78655/new/ https://reviews.llvm.org/D78655 Files: clang/lib/Sema/SemaCUDA.cpp clang/test/CodeGenCUDA/lambda.cu clang/test/SemaCUDA/lambda.cu Index:

[PATCH] D78655: [HIP] Let lambda be host device by default

yaxunl marked 4 inline comments as done. yaxunl added inline comments. Comment at: clang/lib/Sema/SemaCUDA.cpp:722 + if (getLangOpts().HIP) { +Method->addAttr(CUDADeviceAttr::CreateImplicit(Context)); +Method->addAttr(CUDAHostAttr::CreateImplicit(Context)); -

[PATCH] D78655: [HIP] Let lambda be host device by default

tra added a reviewer: rsmith. tra added a subscriber: rsmith. tra added a comment. Summoning @rsmith as I'm sure that there are interesting corner cases in lambda handling that we didn't consider. Making lambdas implicitly HD will make it easier to write the code which can't be instantiated on

[PATCH] D78655: [HIP] Let lambda be host device by default

yaxunl created this revision. yaxunl added reviewers: tra, rjmccall. Lambda functions do not have names, therefore they do not need host/device attribute for overloading resolution. They are also have internal linkage and is only emitted if used, therefore no need to use host/device attribute to i