[AMDGPU] Enable overriding of OpenCL's default address space #117588
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`opencl-def-is-generic-addrspace` sets the default address space from private to generic. This feature allows for building bitcode libraries written in OpenCL that can then be linked against modules compiled from sources written in languages that expect generic as the default address space.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-backend-amdgpu Author: Jakub Chlanda (jchlanda) Changes
Full diff: https://github.com/llvm/llvm-project/pull/117588.diff 7 Files Affected:
diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h
index 9cd23d123f2bac..e2f2c2fafa128d 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -271,6 +271,9 @@ class TargetInfo : public TransferrableTargetInfo,
LLVM_PREFERRED_TYPE(bool)
unsigned AllowAMDGPUUnsafeFPAtomics : 1;
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned OpenCLDefIsGenericAddrSpace : 1;
+
LLVM_PREFERRED_TYPE(bool)
unsigned HasUnalignedAccess : 1;
diff --git a/clang/include/clang/Basic/TargetOptions.h b/clang/include/clang/Basic/TargetOptions.h
index 2049f03b28893f..5e28586ce49c77 100644
--- a/clang/include/clang/Basic/TargetOptions.h
+++ b/clang/include/clang/Basic/TargetOptions.h
@@ -78,6 +78,10 @@ class TargetOptions {
/// \brief If enabled, allow AMDGPU unsafe floating point atomics.
bool AllowAMDGPUUnsafeFPAtomics = false;
+ /// \brief If enabled, allow overriding of the default address space (from
+ /// private to generic).
+ bool OpenCLDefIsGenericAddrSpace = false;
+
/// \brief Code object version for AMDGPU.
llvm::CodeObjectVersionKind CodeObjectVersion =
llvm::CodeObjectVersionKind::COV_None;
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 40fd48761928b3..5f46d417879b79 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5164,6 +5164,13 @@ defm unsafe_fp_atomics : BoolMOption<"unsafe-fp-atomics",
"for certain memory destinations. (AMDGPU only)">,
NegFlag<SetFalse>>;
+defm opencl_def_is_generic_addrspace: BoolMOption<"opencl-def-is-generic-addrspace",
+ TargetOpts<"OpenCLDefIsGenericAddrSpace">, DefaultFalse,
+ PosFlag<SetTrue, [], [ClangOption, CC1Option],
+ "Enable overriding of OpenCL's default address space: from private "
+ "to generic (AMDGPU only).">,
+ NegFlag<SetFalse>>;
+
def faltivec : Flag<["-"], "faltivec">, Group<f_Group>;
def fno_altivec : Flag<["-"], "fno-altivec">, Group<f_Group>;
let Flags = [TargetSpecific] in {
diff --git a/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp
index 86befb1cbc74fc..d6f521895d882d 100644
--- a/clang/lib/Basic/TargetInfo.cpp
+++ b/clang/lib/Basic/TargetInfo.cpp
@@ -157,6 +157,7 @@ TargetInfo::TargetInfo(const llvm::Triple &T) : Triple(T) {
HasAArch64SVETypes = false;
HasRISCVVTypes = false;
AllowAMDGPUUnsafeFPAtomics = false;
+ OpenCLDefIsGenericAddrSpace = false;
HasUnalignedAccess = false;
ARMCDECoprocMask = 0;
diff --git a/clang/lib/Basic/Targets/AMDGPU.cpp b/clang/lib/Basic/Targets/AMDGPU.cpp
index 99f8f2944e2796..e31f22011faeb9 100644
--- a/clang/lib/Basic/Targets/AMDGPU.cpp
+++ b/clang/lib/Basic/Targets/AMDGPU.cpp
@@ -240,6 +240,7 @@ AMDGPUTargetInfo::AMDGPUTargetInfo(const llvm::Triple &Triple,
HasFloat16 = true;
WavefrontSize = (GPUFeatures & llvm::AMDGPU::FEATURE_WAVE32) ? 32 : 64;
AllowAMDGPUUnsafeFPAtomics = Opts.AllowAMDGPUUnsafeFPAtomics;
+ OpenCLDefIsGenericAddrSpace = Opts.OpenCLDefIsGenericAddrSpace;
// Set pointer width and alignment for the generic address space.
PointerWidth = PointerAlign = getPointerWidthV(LangAS::Default);
@@ -262,8 +263,13 @@ void AMDGPUTargetInfo::adjust(DiagnosticsEngine &Diags, LangOptions &Opts) {
// ToDo: There are still a few places using default address space as private
// address space in OpenCL, which needs to be cleaned up, then the references
// to OpenCL can be removed from the following line.
- setAddressSpaceMap((Opts.OpenCL && !Opts.OpenCLGenericAddressSpace) ||
- !isAMDGCN(getTriple()));
+ bool DefaultIsPrivate = (Opts.OpenCL && !Opts.OpenCLGenericAddressSpace) ||
+ !isAMDGCN(getTriple());
+ // Allow for overriding of the default address space in OpenCL (from private
+ // to generic).
+ if (Opts.OpenCL && OpenCLDefIsGenericAddrSpace)
+ DefaultIsPrivate = false;
+ setAddressSpaceMap(DefaultIsPrivate);
}
ArrayRef<Builtin::Info> AMDGPUTargetInfo::getTargetBuiltins() const {
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index d3eec9fea0d498..1f399964fe0ca4 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -7888,6 +7888,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
Args.addOptInFlag(CmdArgs, options::OPT_munsafe_fp_atomics,
options::OPT_mno_unsafe_fp_atomics);
+ Args.addOptInFlag(CmdArgs, options::OPT_mopencl_def_is_generic_addrspace,
+ options::OPT_mno_opencl_def_is_generic_addrspace);
Args.addOptOutFlag(CmdArgs, options::OPT_mamdgpu_ieee,
options::OPT_mno_amdgpu_ieee);
}
diff --git a/clang/test/CodeGen/AMDGPU/opencl_def_is_generic_addrspace.cl b/clang/test/CodeGen/AMDGPU/opencl_def_is_generic_addrspace.cl
new file mode 100644
index 00000000000000..b2c90db28414dc
--- /dev/null
+++ b/clang/test/CodeGen/AMDGPU/opencl_def_is_generic_addrspace.cl
@@ -0,0 +1,18 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 5
+// RUN: %clang_cc1 -triple amdgcn-unknown-amdhsa -emit-llvm -target-cpu gfx900 -mopencl-def-is-generic-addrspace -cl-std=CL3.0 -cl-ext=+__opencl_c_generic_address_space -o - %s | FileCheck -check-prefix=CHECK-OPENCL-DEF-IS-GENERIC %s
+// RUN: %clang_cc1 -triple amdgcn-unknown-amdhsa -emit-llvm -target-cpu gfx900 -cl-std=CL3.0 -cl-ext=+__opencl_c_generic_address_space -o - %s | FileCheck -check-prefix=CHECK-NO-OPENCL-DEF-IS-GENERIC %s
+
+// CHECK-OPENCL-DEF-IS-GENERIC-LABEL: define dso_local void @_Z14funcGenericPtrfPf(
+// CHECK-OPENCL-DEF-IS-GENERIC-SAME: float noundef [[ARGS_0:%.*]], ptr nocapture noundef readnone [[ARGS_1:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// CHECK-OPENCL-DEF-IS-GENERIC-NEXT: [[ENTRY:.*:]]
+// CHECK-OPENCL-DEF-IS-GENERIC-NEXT: ret void
+//
+// CHECK-NO-OPENCL-DEF-IS-GENERIC-LABEL: define dso_local void @_Z14funcGenericPtrfPU3AS0f(
+// CHECK-NO-OPENCL-DEF-IS-GENERIC-SAME: float noundef [[ARGS_0:%.*]], ptr nocapture noundef readnone [[ARGS_1:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// CHECK-NO-OPENCL-DEF-IS-GENERIC-NEXT: [[ENTRY:.*:]]
+// CHECK-NO-OPENCL-DEF-IS-GENERIC-NEXT: ret void
+//
+__attribute__((overloadable)) void funcGenericPtr(float args_0,
+ float __generic *args_1) {
+ return;
+}
|
|
Thank you for this, unfortunately I don't quite see the reason for adding another twiddly bit here. We are long term migrating away from the default-is-private hack, and to get the benefits you are looking for you can simply compile for CL2, or enable the generic as extension, for which there are already defined mechanisms. |
AlexVlx
requested changes
Nov 25, 2024
There was a problem hiding this comment.
In principle I am against this, it adds a relatively brittle hook, and bypasses the pre-existing mechanisms (use CL2 or enable the generic-as extension) for obtaining this behaviour, in a way that does not ensure that the pre-existing mechanisms are available (e.g. it appears one could pass the option, without asking for the generic as extension on CL3.0).
That is fair, after having a second look additional chack for |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
opencl-def-is-generic-addrspacesets the default address space from private to generic. This feature allows for building bitcode libraries written in OpenCL that can then be linked against modules compiled from sources written in languages that expect generic as the default address space.