[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

[PATCH] D95624: [OpenCL][PR48896] Fix default address space in template argument deduction

bader added inline comments. Comment at: clang/test/SemaOpenCLCXX/address-space-templates.cl:60 + // Preserve the address space of the type in forwarding reference. + // CHECK: CXXMethodDecl {{.*}} operator() 'void (__private int &__private) const __generic' + foo4(i,

[PATCH] D97573: [OpenMP] Handle non-function context before checking for diagnostic emission

bader accepted this revision. bader added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97573/new/ https://reviews.llvm.org/D97573 ___

[PATCH] D97717: [SYCL] Rework the SYCL driver options

bader added a comment. @aaron.ballman, it looks like unittests should be updated as well. Please, take a look at failures in pre-merge checks. Comment at: clang/include/clang/Basic/LangOptions.def:252 LANGOPT(SYCLIsDevice , 1, 0, "Generate code for SYCL device")

[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:

[PATCH] D99488: [SYCL][Doc] Add address space handling section to SYCL documentation

bader added a comment. In D99488#2671906 , @Anastasia wrote: > LGTM! Thanks for working on this. The expected sematic seems fairly clear now. Thanks for review! I also fixed external hyperlinks formatting. > We might add a few more details while

[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

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

[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

[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] D100396: [SYCL] Enable `opencl_global_[host,device]` attributes for SYCL

bader created this revision. Herald added subscribers: Naghasan, ldrumm, dexonsmith, kerbowa, Anastasia, ebevhan, yaxunl, nhaehnle, jvesely, jholewinski. bader requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github

[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/

[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 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:

[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 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

[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/

[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 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:

[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

[PATCH] D108020: [NFC] Drop idle compiler option from the test.

bader created this revision. bader added a reviewer: erichkeane. Herald added a subscriber: ebevhan. bader requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D108020 Files:

[PATCH] D108020: [NFC] Drop idle compiler option from the test.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd754b970eddb: [NFC] Drop idle compiler option from the test. (authored by bader). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D108621: [HIPSPV] Add CUDA->SPIR-V address space mapping

bader accepted this revision. bader added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/lib/Basic/Targets/SPIR.h:146 +// See comment on the SPIRDefIsGenMap table. +bool IsHIPSPV = Opts.HIP && Opts.CUDAIsDevice; // FIXME: SYCL

[PATCH] D108621: [HIPSPV] Add CUDA->SPIR-V address space mapping

bader added inline comments. Comment at: clang/lib/Basic/Targets/SPIR.h:59 +// translation). This mapping is enabled when the language mode is HIP. +1, // cuda_device +// cuda_constant pointer can be casted to default/"flat" pointer, but in Anastasia

[PATCH] D108621: [HIPSPV] Add CUDA->SPIR-V address space mapping

bader added inline comments. Comment at: clang/lib/Basic/Targets/SPIR.h:59 +// translation). This mapping is enabled when the language mode is HIP. +1, // cuda_device +// cuda_constant pointer can be casted to default/"flat" pointer, but in keryell

[PATCH] D110281: Change __builtin_sycl_unique_stable_name to just use an Itanium mangling

bader added a comment. LGTM, just one typo in addition to linter reports and I'd like John to take a look. Comment at: clang/docs/LanguageExtensions.rst:2524 mangling scheme at runtime. The mangler marks all the lambdas required to name -the SYCL kernel and emits a stable

[PATCH] D71016: [SYCL] Implement OpenCL kernel function generation

bader added inline comments. Comment at: clang/lib/Sema/SemaSYCL.cpp:45 + /// accessor class. + static bool isSyclAccessorType(const QualType ); + erichkeane wrote: > Isn't there a big rewrite going on downstream of these with > `sycl_special_class`? Why are

[PATCH] D71016: [SYCL] Implement OpenCL kernel function generation

bader added a comment. In D71016#3063762 , @tschuett wrote: > Would a codegenSYCL directory help you to separate Sema from code generation? Moving wrapper kernel function generation to CodeGen library make sense to me. > Doesn't this make AST

[PATCH] D71016: [SYCL] Implement OpenCL kernel function generation

bader added a comment. In D71016#3063457 , @tschuett wrote: > It feels like you are doing codegen(OpenCL kernel) in Sema. Are OpenCL > kernels the only approach. We need to update the description of the patch to clarify that it applies to other GPU

[PATCH] D111566: [SYCL] Fix function pointer address space

bader accepted this revision. bader added a comment. This revision is now accepted and ready to land. @vlastik, your commit fixes function pointers on AVR - https://github.com/llvm/llvm-project/commit/57fd86de879cf2b4c7001b6d0a09df60877ce24d. I suppose this change is required for fixing lvalue

[PATCH] D109144: [SPIR-V] Add SPIR-V triple architecture and clang target info

bader added a comment. In D109144#3042247 , @Anastasia wrote: > 1. Implementing SPIR-V target as SPIR target. @bader do you suggest that we > add `spirv` triple to clang and map it into SPIR taget or do you suggest > something different? What I have

[PATCH] D109818: [HIPSPV] Convert HIP kernels to SPIR-V kernels

bader added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:10224 +// pointers for HIPSPV. When the language mode is HIP, the SPIRTargetInfo +// maps cuda_device to SPIR-V's CrossWorkGroup address space. +llvm::Type *LTy = CGT.ConvertType(Ty);

[PATCH] D108621: [HIPSPV] Add CUDA->SPIR-V address space mapping

bader added inline comments. Comment at: clang/lib/Basic/Targets/SPIR.h:59 +// translation). This mapping is enabled when the language mode is HIP. +1, // cuda_device +// cuda_constant pointer can be casted to default/"flat" pointer, but in Anastasia

[PATCH] D108621: [HIPSPV] Add CUDA->SPIR-V address space mapping

bader added inline comments. Comment at: clang/lib/Basic/Targets/SPIR.h:59 +// translation). This mapping is enabled when the language mode is HIP. +1, // cuda_device +// cuda_constant pointer can be casted to default/"flat" pointer, but in Anastasia

[PATCH] D109144: [SPIR-V] Add SPIR-V triple architecture and clang target info

bader added a comment. In D109144#3032865 , @Anastasia wrote: > It would be good to get closure on this asap. > > @bader We had related discussions on the other reviews about the approach in > this patch. If you have any concerns/suggestions can you

[PATCH] D108621: [HIPSPV] Add CUDA->SPIR-V address space mapping

bader added inline comments. Comment at: clang/lib/Basic/Targets/SPIR.h:59 +// translation). This mapping is enabled when the language mode is HIP. +1, // cuda_device +// cuda_constant pointer can be casted to default/"flat" pointer, but in Anastasia

[PATCH] D109818: [HIPSPV] Convert HIP kernels to SPIR-V kernels

bader added a comment. In D109818#3169531 , @linjamaki wrote: > The patch is ready to land. @Anastasia, @bader, could you commit this patch > to the LLVM for us? Thanks. Could you rebase on the tip of the main branch, please? I see a couple of

[PATCH] D107054: [Clang][CUDA] Add descriptors, mappings, and features for missing CUDA and PTX versions

bader added a comment. Herald added subscribers: carlosgalvezp, asavonic. @tra, ping. @steffenlarsen, does it make sense to add support for recently released 11.5 as well? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107054/new/ https://reviews.llvm.org/D107054

[PATCH] D114080: [SYCL] Diagnose uses of zero length arrays

bader added a comment. LGTM, with a couple of minor suggestions. Comment at: clang/lib/Sema/SemaSYCL.cpp:68-75 +bool ErrorFound = false; +if (isZeroSizedArray(*this, TypeToCheck)) { + SYCLDiagIfDeviceCode(UsedAt, diag::err_sycl_zero_array_size); + ErrorFound

[PATCH] D112404: [SPIR-V] Add translator tool

This revision was automatically updated to reflect the committed changes. Closed by commit rG49682f14bf3f: [SPIR-V] Add translator tool (authored by linjamaki, committed by bader). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112404/new/

[PATCH] D112404: [SPIR-V] Add translator tool

bader accepted this revision. bader added a comment. This revision is now accepted and ready to land. This part looks good to me. Just a couple of minor style comments. Comment at: clang/lib/Driver/ToolChains/SPIRV.cpp:18 + +void SPIRV::constructTranslateCommand(Compilation ,

[PATCH] D109818: [HIPSPV] Convert HIP kernels to SPIR-V kernels

This revision was automatically updated to reflect the committed changes. Closed by commit rG9ae5810b53c2: [HIPSPV] Convert HIP kernels to SPIR-V kernels (authored by linjamaki, committed by bader). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

bader added a comment. In D110622#3174113 , @tra wrote: > The patch looks OK for the time being. That said, I do have concerns that we > may be organically growing something that will be troublesome to deal with > long-term. > > TBH, I still can't

[PATCH] D122587: [clang][NFC] Extract the EmitAssemblyHelper::TargetTriple member

This revision was automatically updated to reflect the committed changes. Closed by commit rG87b28f5092f2: [clang][NFC] Extract the EmitAssemblyHelper::TargetTriple member (authored by psamolysov-intel, committed by bader). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D118935: [SYCL] Disallow explicit casts between mismatching address spaces

bader accepted this revision. bader 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/D118935/new/ https://reviews.llvm.org/D118935

[PATCH] D116266: [SPIR-V] Add linking of separate translation units using spirv-link

bader added inline comments. Comment at: clang/docs/UsersManual.rst:3602 +Linking is done using ``spirv-link`` from `the SPIRV-Tools project +`_. Similar to other external @Anastasia, sorry for late

[PATCH] D114483: [SYCL] Add support for sycl_special_class attribute

bader accepted this revision. bader added a comment. This revision is now accepted and ready to land. LGTM, just one suggestion. It would be great to get @aaron.ballman approve too. Comment at: clang/lib/Sema/SemaDecl.cpp:16690 +

[PATCH] D127579: [clang][WIP] add option to keep types of ptr args for non-kernel functions in metadata

bader added a comment. In D127579#3585516 , @beanz wrote: > @nikic the most important thing you need to know about SPIR-V is that it is a > virtual ISA based on LLVM IR. The ISA itself encodes types for pointers just > like LLVM IR would. And in

[PATCH] D127579: [clang][WIP] add option to keep types of ptr args for non-kernel functions in metadata

bader added a comment. > The way I understand a bitcast instruction in SPIR-V (`OpBitcast` in > https://www.khronos.org/registry/SPIR-V/specs/unified1/SPIRV.html#_conversion_instructions) > is that it can only apply to pointer types which are distinct from function > types. Note that I believe

[PATCH] D136160: [Attr][Doc] Fix pragma unroll documentation.

bader created this revision. bader added a reviewer: aaron.ballman. Herald added a subscriber: ebevhan. Herald added a project: All. bader requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. There is a contradiction in the #pragma unroll

[PATCH] D136160: [Attr][Doc] Fix pragma unroll documentation.

bader updated this revision to Diff 468487. bader added a comment. Update commit message. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136160/new/ https://reviews.llvm.org/D136160 Files: clang/include/clang/Basic/AttrDocs.td Index:

[PATCH] D136160: [Attr][Doc] Fix pragma unroll documentation.

This revision was automatically updated to reflect the committed changes. Closed by commit rG66bd6074c133: [Attr][Doc] Fix pragma unroll documentation. (authored by bader). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136160/new/

[PATCH] D116266: [SPIR-V] Add linking of separate translation units using spirv-link

bader added inline comments. Comment at: clang/docs/UsersManual.rst:3602 +Linking is done using ``spirv-link`` from `the SPIRV-Tools project +`_. Similar to other external Anastasia wrote: > bader wrote: > >

[PATCH] D141375: [SYCL][OpenMP] Fix compilation errors for unsupported __bf16 intrinsics

bader accepted this revision. bader added a comment. This revision is now accepted and ready to land. LGTM. I expect this to be a common issue for all single-source offloading programming models (i.e. CUDA and HIP in addition to SYCL and OpenMP offload). Probably we can generalize the code

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

bader added a comment. @jcranmer-intel, thanks a lot for working on this. I'm so excited to see these changes! Overall, it looks good to me, but I'd like to avoid some runtime computations if possible. Comment at: clang/lib/CodeGen/CGOpenCLRuntime.cpp:40 +static llvm::Type

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

bader added inline comments. Comment at: clang/lib/CodeGen/CGOpenCLRuntime.cpp:40 +static llvm::Type *getSPIRVType(llvm::LLVMContext , StringRef BaseType, +StringRef OpenCLName, StringRef ReadSuffix) { + SmallVector IntParams = {0, 0, 0, 0, 0,

[PATCH] D138284: Fix incorrect cast in VisitSYCLUniqueStableNameExpr

bader added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1635 + Context.getTargetInfo().getConstantAddressSpace().value_or( + LangAS::Default)); llvm::Constant *GlobalConstStr = Builder.CreateGlobalStringPtr( arichardson

[PATCH] D138284: Fix incorrect cast in VisitSYCLUniqueStableNameExpr

bader added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1635 + Context.getTargetInfo().getConstantAddressSpace().value_or( + LangAS::Default)); llvm::Constant *GlobalConstStr = Builder.CreateGlobalStringPtr( > This changes

[PATCH] D138284: Fix incorrect cast in VisitSYCLUniqueStableNameExpr

bader accepted this revision. bader added a comment. This revision is now accepted and ready to land. Thanks for the fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138284/new/ https://reviews.llvm.org/D138284

[PATCH] D142583: [SPIR-V] Add support for __arithmetic_fence builtin for SYCL targets.

bader added a comment. "[SPIR-V] Add support for __arithmetic_fence builtin for SYCL targets." -> "[SPIR] Add support for __arithmetic_fence builtin for SPIR target." Comment at: clang/test/CodeGen/arithmetic-fence-builtin.c:16 +// Test with fast math on spir target +// RUN:

[PATCH] D142583: [SPIR] Add support for __arithmetic_fence builtin for SPIR target.

bader added inline comments. Comment at: clang/test/CodeGen/arithmetic-fence-builtin.c:73 int subit(float a, float b, float *fp) { - // CHECKFAST: define {{.*}}@subit(float noundef %a, float noundef %b{{.*}} + // CHECKPRECISE: define {{.*}}@subit(float noundef %a, float

[PATCH] D142583: [SPIR] Add support for __arithmetic_fence builtin for SPIR target.

bader accepted this revision. bader added a comment. This revision is now accepted and ready to land. LGTM. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142583/new/ https://reviews.llvm.org/D142583 ___ cfe-commits mailing list

[PATCH] D142033: [OpenCL] Always add nounwind attribute for OpenCL

bader accepted this revision. bader added a comment. This revision is now accepted and ready to land. Should we generalize and rename `clang/test/CodeGenOpenCL/convergent.cl` to validate function attributes other than `convergent`? It's not obvious that presence of `nounwind` attribute is

[PATCH] D137154: Adding nvvm_reflect clang builtin

bader added a comment. Is binary size a concern here? NVIDIA, AMD and Intel GPUs are already have ~ 20 different architectures each, so I want my app/library to run on any GPU from these vendors (which is quite reasonable expectation), I'll need to have/distribute ~ 60 different binaries.

[PATCH] D129507: [OffloadPackager] Add option to extract files from images

bader added inline comments. Comment at: clang/test/Driver/offload-packager.c:2-3 +// REQUIRES: x86-registered-target +// REQUIRES: nvptx-registered-target +// REQUIRES: amdgpu-registered-target +// UNSUPPORTED: system-windows Are nvptx and amdgpu target

[PATCH] D147097: [SYCL] Always set NoUnwind attribute for SYCL.

bader added a comment. @hvdijk, thanks a lot for fixing this. In D147097#4229121 , @hvdijk wrote: > Is the rationale I gave in the description correct, or would it be better for > SYCL device code to unconditionally build without `-fexceptions` and get

[PATCH] D147097: [SYCL] Always set NoUnwind attribute for SYCL.

bader accepted this revision. bader 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/D147097/new/ https://reviews.llvm.org/D147097

[PATCH] D151361: [CUDA] bump supported CUDA version to 12.1/11.8

bader added inline comments. Comment at: clang/docs/ReleaseNotes.rst:590 +- Clang now supports CUDA SDK up to 12.1 @tra, could you update llvm/docs/CompileCudaWithLLVM.rst as well, please? Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D151361: [CUDA] bump supported CUDA version to 12.1/11.8

bader added inline comments. Comment at: clang/docs/ReleaseNotes.rst:590 +- Clang now supports CUDA SDK up to 12.1 tra wrote: > bader wrote: > > @tra, could you update llvm/docs/CompileCudaWithLLVM.rst as well, please? > Done in

<    1   2   3   4