[clang] [llvm] [SPIRV][RFC] Rework / extend support for memory scopes (PR #106429)
MrSidims wrote: Tagging @svenvh to be aware of the discussion. I personally don't have a strong opinion at the moment of what should be a default. Currently [SPIR-V To LLVM translator](https://github.com/KhronosGroup/SPIRV-LLVM-Translator) picks "Device" as the default (translation of the atomic scope was added in this [PR](https://github.com/KhronosGroup/SPIRV-LLVM-Translator/pull/)), it's not necessarily correct, but I do believe that SPIR-V Backend and the translator must be aligned, and if Backend picks another default - the translator's implementation must be adjusted. https://github.com/llvm/llvm-project/pull/106429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV][RFC] Rework / extend support for memory scopes (PR #106429)
@@ -251,6 +251,24 @@ SPIRV::MemorySemantics::MemorySemantics getMemSemantics(AtomicOrdering Ord) { llvm_unreachable(nullptr); } +SPIRV::Scope::Scope getMemScope(const LLVMContext &Ctx, SyncScope::ID ID) { + SmallVector SSNs; + Ctx.getSyncScopeNames(SSNs); + + StringRef MemScope = SSNs[ID]; + if (MemScope.empty() || MemScope == "all_svm_devices") MrSidims wrote: I personally prefer to keep an explicit check for whatever default string we have. It should be easier (and hence more error-prone) to reuse names from https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_scope_id instead of thinking about, what is LLVM's default and how it maps on SPIR-V scopes. https://github.com/llvm/llvm-project/pull/106429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV][RFC] Rework / extend support for memory scopes (PR #106429)
https://github.com/MrSidims edited https://github.com/llvm/llvm-project/pull/106429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV][RFC] Rework / extend support for memory scopes (PR #106429)
@@ -58,7 +58,35 @@ class SPIRVTargetCodeGenInfo : public CommonSPIRTargetCodeGenInfo { SPIRVTargetCodeGenInfo(CodeGen::CodeGenTypes &CGT) : CommonSPIRTargetCodeGenInfo(std::make_unique(CGT)) {} void setCUDAKernelCallingConvention(const FunctionType *&FT) const override; + llvm::SyncScope::ID getLLVMSyncScopeID(const LangOptions &LangOpts, + SyncScope Scope, + llvm::AtomicOrdering Ordering, + llvm::LLVMContext &Ctx) const override; }; + +inline StringRef mapClangSyncScopeToLLVM(SyncScope Scope) { + switch (Scope) { + case SyncScope::HIPSingleThread: + case SyncScope::SingleScope: +return "singlethread"; + case SyncScope::HIPWavefront: + case SyncScope::OpenCLSubGroup: + case SyncScope::WavefrontScope: +return "subgroup"; + case SyncScope::HIPWorkgroup: + case SyncScope::OpenCLWorkGroup: + case SyncScope::WorkgroupScope: +return "workgroup"; + case SyncScope::HIPAgent: + case SyncScope::OpenCLDevice: + case SyncScope::DeviceScope: +return "device"; + case SyncScope::SystemScope: + case SyncScope::HIPSystem: + case SyncScope::OpenCLAllSVMDevices: +return "all_svm_devices"; MrSidims wrote: +1, we should align on the scope string names. Since we may have different languages with different naming for the scope it makes sense to take names from the common specification eg SPIR-V. So probably we should rename `sub_group` (or `subgroup`) to `Subgroup`; `singlethread` should be aliased with `Invocation`. https://github.com/llvm/llvm-project/pull/106429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV][RFC] Rework / extend support for memory scopes (PR #106429)
https://github.com/MrSidims edited https://github.com/llvm/llvm-project/pull/106429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV][RFC] Rework / extend support for memory scopes (PR #106429)
https://github.com/MrSidims edited https://github.com/llvm/llvm-project/pull/106429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][llvm][SPIR-V] Explicitly encode native integer widths for SPIR-V (PR #110695)
MrSidims wrote: > There's nothing new to do here. This has always existed @arsenm here is a small experiment, I've compiled the following OpenCL code: ``` struct S { char i8_3[3]; }; kernel void test(global struct S *p, float3 v) { int3 tmp; frexp(v, &tmp); tmp += 1; p->i8_3[0] = tmp.x; p->i8_3[1] = tmp.y; p->i8_3[2] = tmp.z; } ``` with the PR pulled in (on top of LLVM's HEAD aadfba9b2a), the compilation command is: `clang++ -cl-std=CL2.0 -emit-llvm -c -x cl -g0 --target=spir -Xclang -finclude-default-header -O2 test.cl` The output LLVM IR after the optimizations is: ``` ; Function Attrs: convergent norecurse nounwind define dso_local spir_kernel void @test(ptr addrspace(1) nocapture noundef writeonly align 1 %p, <3 x float> noundef %v) local_unnamed_addr #0 !kernel_arg_a> entry: %tmp = alloca <3 x i32>, align 16 call void @llvm.lifetime.start.p0(i64 16, ptr nonnull %tmp) #3 %tmp.ascast = addrspacecast ptr %tmp to ptr addrspace(4) %call = call spir_func <3 x float> @_Z5frexpDv3_fPU3AS4Dv3_i(<3 x float> noundef %v, ptr addrspace(4) noundef %tmp.ascast) #4 %loadVec42 = load <4 x i32>, ptr %tmp, align 16 %extractVec4 = add <4 x i32> %loadVec42, %0 = bitcast <4 x i32> %extractVec4 to i128 %1 = trunc i128 %0 to i96 %2 = bitcast i96 %1 to <12 x i8> %conv = trunc i128 %0 to i8 store i8 %conv, ptr addrspace(1) %p, align 1, !tbaa !9 %conv5 = extractelement <12 x i8> %2, i64 4 %arrayidx7 = getelementptr inbounds i8, ptr addrspace(1) %p, i32 1 store i8 %conv5, ptr addrspace(1) %arrayidx7, align 1, !tbaa !9 %conv8 = extractelement <12 x i8> %2, i64 8 %arrayidx10 = getelementptr inbounds i8, ptr addrspace(1) %p, i32 2 store i8 %conv8, ptr addrspace(1) %arrayidx10, align 1, !tbaa !9 call void @llvm.lifetime.end.p0(i64 16, ptr nonnull %tmp) #3 ret void } ``` note bitcast to i128 with the following truncation to i96 - those types aren't part of the datalayout, yet some optimization generated them. So something has to be done with it and changing the datalayout is not enough. > This does not mean arbitrary integer bitwidths do not work. The n field is > weird, it's more of an optimization hint. Let me clarify myself, _BitInt(N) will work with the change, I have no doubts. But I can imagine a SPIR-V extension to appear that would add support for 4-bit integers. And I can imagine that we would want to not only be able to emit 4-bit integers in the frontend, but also allow optimization passes to emit them. For this it would be nice to have a mechanism that would change datalayout depending on --spirv-ext (or other option). https://github.com/llvm/llvm-project/pull/110695 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][llvm][SPIR-V] Explicitly encode native integer widths for SPIR-V (PR #110695)
MrSidims wrote: > with vanilla upstream, please see You mean the translator, right? I don't think that SPIR-V backend should follow its practices especially when we could do better. > At a glance, it seems like an issue around handling vec3s, which are odd, > but, probably; the BE should probably handle this gracefully rather than > errorring out So we both agree, that the compiler must compile the OpenCL code from above to SPIR-V without erroring out. We can do it in 2 ways: 1. Regularize types in the backend (since the backend is based on top of global isel we should have here better luck then in the translator); 2. Or since this patch modifies datalayout - adjust LLVM pipeline to consider datalayout in optimization passes. The reasoning you have provided in the PR description: "`This is problematic as it leads to optimisation passes, such as InstCombine, getting ideas and e.g. shrinking to non byte-multiple integer types, which is not desirable and can lead to breakage further down in the toolchain.`" totally makes sense to me. So I'm asking if you have plans to go through the passes and modify them, or you only intend to modify InstCombine and/or AMD-specific passes? https://github.com/llvm/llvm-project/pull/110695 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][llvm][SPIR-V] Explicitly encode native integer widths for SPIR-V (PR #110695)
MrSidims wrote: Don't get me wrong, what I'm saying is not an objection against the patch, but rather an attempt to test the waters :) https://github.com/llvm/llvm-project/pull/110695 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][llvm][SPIR-V] Explicitly encode native integer widths for SPIR-V (PR #110695)
https://github.com/MrSidims approved this pull request. https://github.com/llvm/llvm-project/pull/110695 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][llvm][SPIR-V] Explicitly encode native integer widths for SPIR-V (PR #110695)
MrSidims wrote: > InstCombine's primary function is a canonicalization pass. You shouldn't be > modifying it for specifically SPIRV optimizations (with the exception of > SPIRV intrinsic support). SPIRV specific transforms belong in later backend > IR passes Does it mean, that the reasoning behind this very PR is not legit? > This is problematic as it leads to optimisation passes, such as InstCombine, > getting ideas and e.g. shrinking to non byte-multiple integer types, which is > not desirable and can lead to breakage further down in the toolchain. https://github.com/llvm/llvm-project/pull/110695 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][llvm][SPIR-V] Explicitly encode native integer widths for SPIR-V (PR #110695)
https://github.com/MrSidims commented: Thanks, it should make LLVM IR after optimizations more translatable in SPIR-V! Few questions though: 1. Usually (or at least AFAIK) optimization passes won't consider datalayout automatically, as LLVM defines datalayout not as a contract set by the frontend, but a contact, that the code generator expects. Do you plan to go over LLVM passes adding this check? 2. Some existing and future extensions might allow extra bit widths for integers. For example here is [SPV_INTEL_arbitrary_precision_integers](https://github.com/KhronosGroup/SPIRV-Registry/blob/main/extensions/INTEL/SPV_INTEL_arbitrary_precision_integers.asciidoc) extension that allows any bit widths for integers (it's actually a bad example as it's developed only for _BitInt C23 extension and FPGA hardware, so datalayout wouldn't have an impact on it) or one of the internally discussed within Khronos extensions for ML (which might be impacted by this change). Can we envision, how can we change datalayout information depending on the enabled extensions (or you don't think it's a big problem?) https://github.com/llvm/llvm-project/pull/110695 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][llvm][SPIR-V] Explicitly encode native integer widths for SPIR-V (PR #110695)
MrSidims wrote: > You want spirv, not spir Thanks! Yet the result is the same. > Do you plan to go over LLVM passes adding this check? So guess answer to my question would be: "no" :) https://github.com/llvm/llvm-project/pull/110695 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][Driver] Add HIPAMD Driver support for AMDGCN flavoured SPIR-V (PR #95061)
@@ -193,6 +194,33 @@ void AMDGCN::Linker::constructLldCommand(Compilation &C, const JobAction &JA, Lld, LldArgs, Inputs, Output)); } +// For SPIR-V the inputs for the job are device AMDGCN SPIR-V flavoured bitcode +// and the output is either a compiled SPIR-V binary or bitcode (-emit-llvm). It +// calls llvm-link and then the llvm-spirv translator. Once the SPIR-V BE will +// be promoted from experimental, we will switch to using that. TODO: consider +// if we want to run any targeted optimisations over IR here, over generic +// SPIR-V. +void AMDGCN::Linker::constructLinkAndEmitSpirvCommand( +Compilation &C, const JobAction &JA, const InputInfoList &Inputs, +const InputInfo &Output, const llvm::opt::ArgList &Args) const { + assert(!Inputs.empty() && "Must have at least one input."); + + constructLlvmLinkCommand(C, JA, Inputs, Output, Args); + + // Linked BC is now in Output + + // Emit SPIR-V binary. + llvm::opt::ArgStringList TrArgs{ + "--spirv-max-version=1.6", + "--spirv-ext=+all", + "--spirv-allow-extra-diexpressions", + "--spirv-allow-unknown-intrinsics", + "--spirv-lower-const-expr", + "--spirv-preserve-auxdata", + "--spirv-debug-info-version=nonsemantic-shader-200"}; MrSidims wrote: Recently I've found this patch in gitlog and was intrigued, does this line mean, that AMD driver supports https://github.com/KhronosGroup/SPIRV-Registry/pull/186 ? Just for my curiosity. It may also make me push the instruction set for ratification sooner :) https://github.com/llvm/llvm-project/pull/95061 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][Driver] Add HIPAMD Driver support for AMDGCN flavoured SPIR-V (PR #95061)
@@ -193,6 +194,33 @@ void AMDGCN::Linker::constructLldCommand(Compilation &C, const JobAction &JA, Lld, LldArgs, Inputs, Output)); } +// For SPIR-V the inputs for the job are device AMDGCN SPIR-V flavoured bitcode +// and the output is either a compiled SPIR-V binary or bitcode (-emit-llvm). It +// calls llvm-link and then the llvm-spirv translator. Once the SPIR-V BE will +// be promoted from experimental, we will switch to using that. TODO: consider +// if we want to run any targeted optimisations over IR here, over generic +// SPIR-V. +void AMDGCN::Linker::constructLinkAndEmitSpirvCommand( +Compilation &C, const JobAction &JA, const InputInfoList &Inputs, +const InputInfo &Output, const llvm::opt::ArgList &Args) const { + assert(!Inputs.empty() && "Must have at least one input."); + + constructLlvmLinkCommand(C, JA, Inputs, Output, Args); + + // Linked BC is now in Output + + // Emit SPIR-V binary. + llvm::opt::ArgStringList TrArgs{ + "--spirv-max-version=1.6", + "--spirv-ext=+all", + "--spirv-allow-extra-diexpressions", MrSidims wrote: @AlexVlx nit: if generation of NonSemantic.Shader.DebugInfo.200 is turned on - this option is not needed as the extended instruction already adds all DWARF expressions (including LLVM-specific expressions). https://github.com/llvm/llvm-project/pull/95061 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIR-V] Fixup storage class for global private (PR #116636)
Nathan =?utf-8?q?Gau=C3=ABr?= Message-ID: In-Reply-To: https://github.com/MrSidims approved this pull request. LGTM Guess the PR description should be updated https://github.com/llvm/llvm-project/pull/116636 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIR-V] Fixup storage class for global private (PR #116636)
Nathan =?utf-8?q?Gauër?= Message-ID: In-Reply-To: https://github.com/MrSidims edited https://github.com/llvm/llvm-project/pull/116636 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver][SPIR-V] Use consistent tools to convert between text and binary form (PR #120266)
MrSidims wrote: I agree with Sven. We mustn't reuse llvm-spirv's tests format in other repositories. https://github.com/llvm/llvm-project/pull/120266 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver][SPIR-V] Use consistent tools to convert between text and binary form (PR #120266)
https://github.com/MrSidims approved this pull request. https://github.com/llvm/llvm-project/pull/120266 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC][clang][HIP] Remove flag from SPIR-V Translator invocation (PR #122995)
https://github.com/MrSidims approved this pull request. https://github.com/llvm/llvm-project/pull/122995 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][SPIR-V] Use the SPIR-V backend by default (PR #129545)
https://github.com/MrSidims approved this pull request. https://github.com/llvm/llvm-project/pull/129545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV] GPU intrinsics (PR #131190)
@@ -0,0 +1,501 @@ +//===- LowerGPUIntrinsic.cpp --===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// +// +// Lower the llvm.gpu intrinsics to target specific code sequences. +// Can be called from clang if building for a specific GPU or from the backend +// as part of a SPIRV lowering pipeline. Initial pass can lower to amdgcn or MrSidims wrote: First of all I agree, that having a common interface between various frontends is a good idea. Yet as from what I see now - the intrinsics added in the PR are not common, but tuned only for AMDGPU and NVPTX. And just like @michalpaszkowski I don't understand, what place SPIR-V has for those intrinsics. >The spirv backend should stash these intrinsics in the SPV file like any other >ones. It might need a patch to do so or we might get default handling for llvm >prefixed intrinsics. That's the point really - we pass information about the >SIMT computation through the SPV onward to whatever is dealing with that later. >I don't know what distinction you're drawing between abstract function calls >and intrinsics, terminology is not consistent across compilers here. Please correct me if I understood this part incorrectly. Is your proposal to store `llvm.gpu` intrinsic call in SPIR-V module as is if there is no appropriate SPIR-V instruction for it? https://github.com/llvm/llvm-project/pull/131190 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV] GPU intrinsics (PR #131190)
https://github.com/MrSidims edited https://github.com/llvm/llvm-project/pull/131190 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV] GPU intrinsics (PR #131190)
https://github.com/MrSidims edited https://github.com/llvm/llvm-project/pull/131190 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV] GPU intrinsics (PR #131190)
https://github.com/MrSidims edited https://github.com/llvm/llvm-project/pull/131190 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV] GPU intrinsics (PR #131190)
https://github.com/MrSidims edited https://github.com/llvm/llvm-project/pull/131190 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV] GPU intrinsics (PR #131190)
@@ -0,0 +1,501 @@ +//===- LowerGPUIntrinsic.cpp --===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// +// +// Lower the llvm.gpu intrinsics to target specific code sequences. +// Can be called from clang if building for a specific GPU or from the backend +// as part of a SPIRV lowering pipeline. Initial pass can lower to amdgcn or MrSidims wrote: > I'd like lowering for intel too but that will need to be out of tree until > intel comes in tree. To clarify: while me and Michal are working for Intel, our concerns are not about lowering to some Intel's internal stuff, but about lowering to SPIR-V, which is supported as intermediate representation for Vulkan, OpenCL etc by multiple vendors. I can't speak for others, but (judging by amount of PRs I've been looking in LLVM's SPIR-V backend) apart of Intel at least Google has interest in developing SPIR-V backend. So let me actually invite @sudonatalie and @Keenuts to the discussion. > which passes llvm intrinsics through unchanged, and ends up creating amdgcn-- > LLVM IR to feed back into the backend I assume it only works, when the frontend and the backend are built on top of the same LLVM version, right? Otherwise you would face an issues like: a. intrinsics semantics can change depending on LLVM version; b. rules of intrinsics' name can change from version to version (for example LLVM's switch from typed to untyped pointers has affected the names). So talking about SPIR-V toolchains, as it's hard to know ahead of time, who will consume the generated SPIR-V - we must not have compiler generated external symbols (like intrinsics) in the module as the backend might not be able to resolve them. https://github.com/llvm/llvm-project/pull/131190 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV] GPU intrinsics (PR #131190)
https://github.com/MrSidims edited https://github.com/llvm/llvm-project/pull/131190 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV] GPU intrinsics (PR #131190)
https://github.com/MrSidims edited https://github.com/llvm/llvm-project/pull/131190 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][OpenCL][AMDGPU] Allow a kernel to call another kernel (PR #115821)
MrSidims wrote: @lalaniket8 @arsenm I don't have a strong opinion, but shouldn't this transformation be done during lowering to the target? Current version of the patch brings odd behavior for LLVM IR to SPIR-V lowering for OpenCL kernels. SPIR-V don't allow one EntryPoint to refer another EntryPoint, so during such lowering the translator moves kernel's body to impl function (just like this patch does). Together it results in quite odd behavior: https://github.com/KhronosGroup/SPIRV-LLVM-Translator/issues/3115 https://github.com/llvm/llvm-project/pull/115821 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU][clang][CodeGen][opt] Add late-resolved feature identifying predicates (PR #134016)
MrSidims wrote: > Thank you for the feedback! I might not be getting the question right (case > in which I apologise in advance!), but I think that for "vanilla" SPIR-V i.e. > not vendor flavoured one, where one strictly has to deal with Extensions / > non-core capabilities, we probably would have the following situation: I was imagining cases like this: ``` if (__builtin_amdgcn_processor_is("some_hw_with_fp16_support) { /*code using fp16*/ } else { /*code using fp32*/ } ``` note, that when translated to SPIR-V the SPIR-V generator must insert **Float16** capability (in the beginning of the module). So such tool would need to remove that capability as well. A side question, is it legal to use the builtin in unstructured control flow, like here: https://godbolt.org/z/qnhKdhfdW ? https://github.com/llvm/llvm-project/pull/134016 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [Driver] Report invalid target triple versions for all environment types. (PR #78655)
MrSidims wrote: Hi @ZijunZhaoCCK , sorry, I'm quite late for the party, but may I ask you some questions? We want to be able to distinguish between Vulkan and OpenCL environments in [SPIR-V backend](https://github.com/intel/llvm/tree/sycl/llvm/lib/Target/SPIRV), so we could be able to pick between Shader and Kernel capabilities depending on the environment. And OpenCL environment added by this patch seems like the perfect match for us. We would also need to change OpenCL CodeGen in clang to start emitting this part of the triple, but before doing that I'd like to check, what was the intention under adding OpenCL environment in this PR (as currently it's not exposed in LLVM anywhere, so I could take a look)? Particularly I'd like to know, whether your intentions were to add OpenCL as a language to the triple or OpenCL as an execution environment? https://github.com/llvm/llvm-project/pull/78655 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][OpenCL][AMDGPU] Allow a kernel to call another kernel (PR #115821)
MrSidims wrote: > this change makes sense and it doesn't bring (known to me) regressions Actually, there is an incorrect behavior in the following test case: https://godbolt.org/z/dc3T7Mo3G , note __clang_ocl_kern_imp_sample_kernel_float was generated, but was never called. @lalaniket8 can this be addressed? https://github.com/llvm/llvm-project/pull/115821 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][OpenCL][AMDGPU] Allow a kernel to call another kernel (PR #115821)
MrSidims wrote: > This is the lowering to the target. My glossary might not be lacking some definitions, but what I really meant by lowering is: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU or https://github.com/llvm/llvm-project/tree/main/llvm/lib/Target/SPIRV . I definitely see a value of having this resolved in one place instead of multiple places, and clang is a good candidate for it. On the other hand: a. some targets don't have such restriction; 2. clang is not a single frontend, especially now - in MLIR world, does it mean, what every frontend should make such adjustment when lowering to AMDGPU or SPIR-V? Just a note/disclaimer, this change makes sense and it doesn't bring (known to me, the reason I'm here is a surprise to see [this](https://godbolt.org/z/zW84fdace) during my experiments) regressions in some downsteam and adjusting tests in the translator is totally fine for me. What I'm trying to understand if it's really should be done in clang for OpenCL for every target. https://github.com/llvm/llvm-project/pull/115821 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][OpenMP][SPIR-V] Fix AS of globals and set the default AS to 4 (PR #135251)
@@ -0,0 +1,23 @@ +// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -fopenmp-targets=spirv64 -emit-llvm-bc %s -o %t-host.bc +// RUN: %clang_cc1 -fopenmp -fopenmp-targets=spirv64 -fopenmp-is-target-device -triple spirv64 -fopenmp-host-ir-file-path %t-host.bc -emit-llvm %s -o - | FileCheck %s + +int main() { + int x = 0; + +#pragma omp target teams distribute parallel for simd + for(int i = 0; i < 1024; i++) +x+=i; + return x; +} + +// CHECK: @[[#STRLOC:]] = private unnamed_addr addrspace(1) constant [{{.*}} x i8] c{{.*}}, align 1 +// CHECK: @[[#IDENT:]] = private unnamed_addr addrspace(1) constant %struct.ident_t { i32 {{.*}}, i32 2050, i32 {{.*}}, i32 {{.*}}, ptr addrspacecast (ptr addrspace(1) @[[#STRLOC]] to ptr) }, align 8 +// CHECK: define internal spir_func void @__omp_offloading_{{.*}}_omp_outlined(ptr addrspace(4) noalias noundef {{.*}}., ptr addrspace(4) noalias noundef {{.*}}, i64 noundef {{.*}}) #{{.*}} { +// CHECK: = load ptr addrspace(4), ptr addrspace(4) %{{.*}}, align 8 +// CHECK: = load i32, ptr addrspace(4) %{{.*}}, align 4 +// CHECK: = addrspacecast ptr addrspace(4) %{{.*}} to ptr +// CHECK: = addrspacecast ptr addrspace(4) %{{.*}} to ptr +// CHECK: = addrspacecast ptr addrspace(4) %{{.*}} to ptr +// CHECK: = addrspacecast ptr addrspace(4) %{{.*}} to ptr +// CHECK: call spir_func void @__kmpc_distribute_static_init{{.*}}(ptr addrspacecast (ptr addrspace(1) @[[#IDENT]] to ptr), i32 %{{.*}}, i32 {{.*}}, ptr %{{.*}}, ptr %{{.*}}, ptr %{{.*}}, ptr %{{.*}}, i32 {{.*}}, i32 %{{.*}}) MrSidims wrote: This looks wrong. Casts from global to private are not allowed in SPIR and SPIR-V. https://github.com/llvm/llvm-project/pull/135251 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][SPIR-V] Addrspace of opencl_global should always be 1 (PR #136753)
https://github.com/MrSidims approved this pull request. https://github.com/llvm/llvm-project/pull/136753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits