[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. bader marked an inline comment as done. Closed by commit rG7818906ca134: [SYCL] Implement SYCL address space attributes handling (authored by bader). Changed prior to

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

aaron.ballman accepted this revision. aaron.ballman added a comment. Just one drive-by nit from me. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4119-4123 + if (LangOpts.SYCLIsDevice) { +if (!D || D->getType().getAddressSpace() == LangAS::Default) { + return LangAS

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia accepted this revision. Anastasia added a comment. This revision is now accepted and ready to land. LGTM! Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89909/new/ https://reviews.llvm.org/D89909 __

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader marked 5 inline comments as done. bader added a comment. @Anastasia, I've updated https://reviews.llvm.org/D99488 and refactored `getStringLiteralAddressSpace` to handle non-string constants as well. Please, take a look. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader updated this revision to Diff 339973. bader marked an inline comment as done. bader added a comment. Generalize getStringLiteralAddressSpace to GetGlobalConstantAddressSpace Rebased on ToT. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89909/

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader marked 2 inline comments as done. bader added inline comments. Comment at: clang/include/clang/AST/Type.h:493 + // Default is a superset of SYCL address spaces. + (A == LangAS::Default && +(B == LangAS::sycl_private || B == LangAS::sycl_local

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added inline comments. Comment at: clang/include/clang/AST/Type.h:493 + // Default is a superset of SYCL address spaces. + (A == LangAS::Default && +(B == LangAS::sycl_private || B == LangAS::sycl_local || bader wrote: >

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader marked an inline comment as done. bader added inline comments. Comment at: clang/lib/Basic/Targets/SPIR.h:140 +// space must be compatible with the generic address space +return LangAS::sycl_global; + } Anastasia wrote: > This needs a language guar

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader updated this revision to Diff 339515. bader marked an inline comment as done. bader added a comment. Added SYCL address spaces mangling for targets without address space map Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89909/new/ https://rev

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader updated this revision to Diff 339484. bader marked 7 inline comments as done. bader added a comment. Applied more review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89909/new/ https://reviews.llvm.org/D89909 Files: clang/includ

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:9985 + "Address space agnostic languages only"); + LangAS DefaultGlobalAS = getLangASFromTargetAS( + CGM.getContext().getTargetAddressSpace(LangAS::sycl_global)); bader wr

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:9985 + "Address space agnostic languages only"); + LangAS DefaultGlobalAS = getLangASFromTargetAS( + CGM.getContext().getTargetAddressSpace(LangAS::sycl_global)); Anastasia wr

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:9985 + "Address space agnostic languages only"); + LangAS DefaultGlobalAS = getLangASFromTargetAS( + CGM.getContext().getTargetAddressSpace(LangAS::sycl_global)); bader wr

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:9985 + "Address space agnostic languages only"); + LangAS DefaultGlobalAS = getLangASFromTargetAS( + CGM.getContext().getTargetAddressSpace(LangAS::sycl_global)); Anastasia wr

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:9985 + "Address space agnostic languages only"); + LangAS DefaultGlobalAS = getLangASFromTargetAS( + CGM.getContext().getTargetAddressSpace(LangAS::sycl_global)); bader wr

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:9985 + "Address space agnostic languages only"); + LangAS DefaultGlobalAS = getLangASFromTargetAS( + CGM.getContext().getTargetAddressSpace(LangAS::sycl_global)); Anastasia wr

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added inline comments. Comment at: clang/lib/AST/ItaniumMangle.cpp:2379 unsigned TargetAS = Context.getASTContext().getTargetAddressSpace(AS); - if (TargetAS != 0) + if (TargetAS != 0 || Context.getASTContext().getLangOpts().SYCLIsDevice) ASStr

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader added inline comments. Comment at: clang/lib/AST/ItaniumMangle.cpp:2379 unsigned TargetAS = Context.getASTContext().getTargetAddressSpace(AS); - if (TargetAS != 0) + if (TargetAS != 0 || Context.getASTContext().getLangOpts().SYCLIsDevice) ASString

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader updated this revision to Diff 337441. bader marked 5 inline comments as done. bader added a comment. Applied more comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89909/new/ https://reviews.llvm.org/D89909 Files: clang/include/clang

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added inline comments. Comment at: clang/lib/AST/ItaniumMangle.cpp:2379 unsigned TargetAS = Context.getASTContext().getTargetAddressSpace(AS); - if (TargetAS != 0) + if (TargetAS != 0 || Context.getASTContext().getLangOpts().SYCLIsDevice) ASStr

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader marked 2 inline comments as done. bader added inline comments. Comment at: clang/include/clang/AST/Type.h:488 (A == LangAS::opencl_global && (B == LangAS::opencl_global_device || B == LangAS::opencl_global_host)) ||

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader added inline comments. Comment at: clang/include/clang/AST/Type.h:493 + // Default is a superset of SYCL address spaces. + (A == LangAS::Default && +(B == LangAS::sycl_private || B == LangAS::sycl_local || Anastasia wrote: >

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader updated this revision to Diff 337180. bader marked 16 inline comments as done. bader added a comment. Applied more code review suggestions. Rebased on ToT. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89909/new/ https://reviews.llvm.org/D89

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added inline comments. Comment at: clang/include/clang/AST/Type.h:488 (A == LangAS::opencl_global && (B == LangAS::opencl_global_device || B == LangAS::opencl_global_host)) || // Consider pointer size a

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader marked an inline comment as done. bader added inline comments. Comment at: clang/include/clang/AST/Type.h:488 (A == LangAS::opencl_global && (B == LangAS::opencl_global_device || B == LangAS::opencl_global_host)) ||

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added inline comments. Comment at: clang/test/CodeGenSYCL/address-space-parameter-conversions.cpp:80 + // Ensure that we still get 3 different template instantiations. + tmpl(GLOB); + // CHECK-DAG: [[GLOB_LOAD4:%[a-zA-Z0-9]+]] = load i32 addrspace(1)*, i32 addrspace

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added inline comments. Comment at: clang/include/clang/AST/Type.h:493 + // Default is a superset of SYCL address spaces. + (A == LangAS::Default && +(B == LangAS::sycl_private || B == LangAS::sycl_local || bader wrote: >

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader added inline comments. Comment at: clang/include/clang/AST/Type.h:493 + // Default is a superset of SYCL address spaces. + (A == LangAS::Default && +(B == LangAS::sycl_private || B == LangAS::sycl_local || Anastasia wrote: >

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader updated this revision to Diff 336543. bader marked 32 inline comments as done. bader added a comment. Applied code review suggestions. Rebased on ToT and updated commit message. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89909/new/ https:

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added inline comments. Comment at: clang/include/clang/AST/Type.h:493 + // Default is a superset of SYCL address spaces. + (A == LangAS::Default && +(B == LangAS::sycl_private || B == LangAS::sycl_local || Ok if you allow

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added a comment. In D89909#2653454 , @bader wrote: >> Yesterday, we chatted offline and agreed that the main issue is missing >> documentation for Clang extensions being added for SYCL. To address this >> issue we are adding SYCL architecture d

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader added a comment. > Yesterday, we chatted offline and agreed that the main issue is missing > documentation for Clang extensions being added for SYCL. To address this > issue we are adding SYCL architecture design document, which we are going to > update along with adding new features. I'

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added a comment. In D89909#2652459 , @bader wrote: > In D89909#2624284 , @aaron.ballman > wrote: > >> Based on the discussion so far, would these be acceptable steps to take? >> >> 0) Complete review on t

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader added a comment. In D89909#2624284 , @aaron.ballman wrote: > Based on the discussion so far, would these be acceptable steps to take? > > 0) Complete review on this patch for any technical concerns related to it and > commit when it's ready (this u

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added a comment. > Having multiple maps is not something new to the clang community. Similar > approach AMDGPU target applies to customize address space mapping for OpenCL > language > (https://github.com/llvm/llvm-project/blob/main/clang/lib/Basic/Targets/AMDGPU.cpp#L354). > > I up

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader added a comment. Let's take a look at compilation output for the following program: c++ void foo(int *p) {} OpenCL compiler produces following LLVM IR for SPIR target: LLVM define dso_local spir_func void @_Z3fooPU3AS4i(i32 addrspace(4)* nocapture %0) local_unnamed_addr #0 {

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader updated this revision to Diff 333247. bader added a comment. [NFC] Align address space map names with AMDGPU target. Rename SPIRAddrSpaceMap and SYCLAddrSpaceMap to SPIRDefIsPrivMap and SPIRDefIsGenMap. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added a comment. > If what you're suggesting is that Clang have a SYCL-specific page that serves > a similar purpose to https://clang.llvm.org/docs/OpenCLSupport.html, then I > agree we should have that (and it should be linked to from > https://clang.llvm.org/docs/index.html the same

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

aaron.ballman added a comment. In D89909#2623887 , @Anastasia wrote: > In D89909#2623250 , @aaron.ballman > wrote: > >> In D89909#2623211 , @Anastasia >> wrote: >> >>> In D

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added a comment. In D89909#2623250 , @aaron.ballman wrote: > In D89909#2623211 , @Anastasia wrote: > >> In D89909#2617194 , @bader wrote: >> >>> @Anastasia, do you

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

aaron.ballman added a comment. In D89909#2623211 , @Anastasia wrote: > In D89909#2617194 , @bader wrote: > >> @Anastasia, do you suggest we copy >> https://github.com/intel/llvm/blob/sycl/sycl/doc/CompilerAndRuntim

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added a comment. In D89909#2617194 , @bader wrote: > In D89909#2606180 , @Anastasia wrote: > >> In D89909#2600859 , @aaron.ballman >> wrote: >> >>> Just a few mino

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader added a comment. In D89909#2606180 , @Anastasia wrote: > In D89909#2600859 , @aaron.ballman > wrote: > >> Just a few minor nits from me, but I'm mostly wondering: where are we at >> with this and are there s

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added a comment. In D89909#2600859 , @aaron.ballman wrote: > Just a few minor nits from me, but I'm mostly wondering: where are we at with > this and are there still substantive changes required? (I looked through the > comments, but there's a

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader updated this revision to Diff 328100. bader marked 4 inline comments as done. bader added a comment. Apply suggestions from Aaron. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89909/new/ https://reviews.llvm.org/D89909 Files: clang/includ

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

aaron.ballman added a comment. Just a few minor nits from me, but I'm mostly wondering: where are we at with this and are there still substantive changes required? (I looked through the comments, but there's a lot of back-and-forth since Oct and I'm not certain what's holding the patch back cur

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added a comment. > Sorry, I forgot to mentioned that this change was also discussed in the > mailing list: > http://clang-developers.42468.n3.nabble.com/RFC-Re-use-OpenCL-address-space-attributes-for-SYCL-td4068754.html. > There are no objections from the community. I remember asking

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader added a comment. > Anastasia added a comment. > In D89909#2536331 > https://reviews.llvm.org/D89909#2536331, @bader wrote: > >> In D89909#2536157 >> https://reviews.llvm.org/D89909#2536157, @Anastasia wrote

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added a comment. In D89909#2536331 , @bader wrote: > In D89909#2536157 , @Anastasia wrote: > >> In D89909#2534790 , @bader wrote: >> Regarding SYCLDevice and S

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader updated this revision to Diff 320761. bader added a comment. Fixed a couple of typos in the comments; NFC. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89909/new/ https://reviews.llvm.org/D89909 Files: clang/include/clang/AST/Type.h cla

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader marked an inline comment as done. bader added a comment. In D89909#2536157 , @Anastasia wrote: > In D89909#2534790 , @bader wrote: > >>> Regarding SYCLDevice and SYCLAddrSpaceMap I am still not very convinced

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader updated this revision to Diff 320741. bader added a comment. Applied code review suggestions from Anastasia. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89909/new/ https://reviews.llvm.org/D89909 Files: clang/include/clang/AST/Type.h c

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added a comment. In D89909#2534790 , @bader wrote: >> Regarding SYCLDevice and SYCLAddrSpaceMap I am still not very convinced >> about the flow. Have you had any design discussion regarding this already >> that you could point to? > > We discus

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader added a subscriber: svenvh. bader added a comment. > Regarding SYCLDevice and SYCLAddrSpaceMap I am still not very convinced about > the flow. Have you had any design discussion regarding this already that you > could point to? We discussed this with you in https://github.com/intel/llvm/p

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added a comment. In D89909#2496269 , @bader wrote: > In D89909#2448443 , @Anastasia wrote: > >> If you prefer to continue this route then you should just document your >> dialect sematic somewhere publicl

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader added a subscriber: aaron.ballman. bader added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:9996 + if (CGM.isTypeConstant(D->getType(), false)) { +if (auto ConstAS = CGM.getTarget().getConstantAddressSpace()) + return ConstAS.getValue(); -

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader updated this revision to Diff 319566. bader added a comment. Rebase patch on ToT and applied small refactoring. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89909/new/ https://reviews.llvm.org/D89909 Files: clang/include/clang/AST/Type.h

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader added a comment. Ping^2. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89909/new/ https://reviews.llvm.org/D89909 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89909/new/ https://reviews.llvm.org/D89909 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bi

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader updated this revision to Diff 316447. bader added a comment. Re-base patch and fix lit test failures. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89909/new/ https://reviews.llvm.org/D89909 Files: clang/include/clang/AST/Type.h clang/in

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader added a comment. In D89909#2448443 , @Anastasia wrote: > If you prefer to continue this route then you should just document your > dialect sematic somewhere publicly accessible and avoid aliasing to OpenCL, > or target address spaces. So it would b

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added a comment. In D89909#2442573 , @bader wrote: > In D89909#2439837 , @Anastasia wrote: > >> In D89909#2423750 , @bader wrote: >> It was mentioned that the

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader added a comment. In D89909#2439837 , @Anastasia wrote: > In D89909#2423750 , @bader wrote: > >>> It was mentioned that the changes in the type system with address spaces is >>> undesirable for SYCL because yo

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added a comment. In D89909#2423750 , @bader wrote: >> It was mentioned that the changes in the type system with address spaces is >> undesirable for SYCL because you indend to parse existing C++ code as-is. >> This contradicts the intended sema

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader updated this revision to Diff 310003. bader added a comment. Move CodeGen crash fix to a separate review request: https://reviews.llvm.org/D92782 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89909/new/ https://reviews.llvm.org/D89909 Files

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:4596 + IRFuncTy->getParamType(FirstIRArg)); +} + bader wrote: > bader wrote: > > rjmccall wrote: > > > This seems problematic; code like this

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:4596 + IRFuncTy->getParamType(FirstIRArg)); +} + bader wrote: > rjmccall wrote: > > This seems problematic; code like this shouldn't be neces

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader updated this revision to Diff 309357. bader added a comment. Apply code review comment from @rjmccall Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89909/new/ https://reviews.llvm.org/D89909 Files: clang/include/clang/AST/Type.h clang/in

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader added a comment. > I didn't sense any agreement or any sort of a conclusion on the RFC. > Moreover, I didn't feel at all that the existing address space attribute was > a good fit. I was suggesting to add a different attribute that isn't an > address space attribute from Embedded C. I don

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added a comment. In D89909#2403069 , @bader wrote: >> Did anyone conclude there that the language address spaces should be added >> for SYCL? I can't see any of this. In fact I don't even think there was any >> conclusion on the RFC. You should

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader added a comment. > Did anyone conclude there that the language address spaces should be added > for SYCL? I can't see any of this. In fact I don't even think there was any > conclusion on the RFC. You should first make your design clear and agreed > before going ahead with the implementat

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:4554 + + // Language rules define if it is legal to cast from one address space + // to another, and which address space we should use as a "common asavonic wrote: > bader wrote: > >

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added a comment. In D89909#2388622 , @bader wrote: > In D89909#2353931 , @Anastasia wrote: > >> In the RFC it has been discussed to either use target address spaces or >> perhaps to introduce a new attrib

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader updated this revision to Diff 305167. bader added a comment. Upload full patch after removing `sycl_constant` address space attribute support. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89909/new/ https://reviews.llvm.org/D89909 Files:

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader updated this revision to Diff 305165. bader added a comment. Remove support for `sycl_constant` address space attribute. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89909/new/ https://reviews.llvm.org/D89909 Files: clang/include/clang/Ba

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

asavonic added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:4554 + + // Language rules define if it is legal to cast from one address space + // to another, and which address space we should use as a "common bader wrote: > asavonic wrote: >

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:4554 + + // Language rules define if it is legal to cast from one address space + // to another, and which address space we should use as a "common asavonic wrote: > sdmitriev wrote:

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

asavonic added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:4554 + + // Language rules define if it is legal to cast from one address space + // to another, and which address space we should use as a "common sdmitriev wrote: > bader wrote:

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

sdmitriev added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:4554 + + // Language rules define if it is legal to cast from one address space + // to another, and which address space we should use as a "common bader wrote: > Anastasia wrote:

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader added a subscriber: sdmitriev. bader added a comment. In D89909#2353931 , @Anastasia wrote: > In the RFC it has been discussed to either use target address spaces or > perhaps to introduce a new attribute to reflect a semantic needed for SYCL, > bu

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Anastasia added a comment. In the RFC it has been discussed to either use target address spaces or perhaps to introduce a new attribute to reflect a semantic needed for SYCL, but it seems to me in this change you are building on top of the existing language address space attribute with new entr

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

rjmccall added inline comments. Comment at: clang/lib/Basic/Targets/AMDGPU.cpp:57 +Constant, // sycl_constant +Private, // sycl_private Generic, // ptr32_sptr I feel like we may be outgrowing these address-space map arrays. Comme

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

bader created this revision. bader added reviewers: Anastasia, keryell, Naghasan, asavonic, Fznamznon. Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, kerbowa, ebevhan, hiraditya, yaxunl, nhaehnle, jvesely, jholewinski. Herald added projects: clang, LLVM. bader requested review of