[clang][OMPIRBuilder] Use default target AS to create types #158152

sarnex · 2025-09-11T21:19:36Z

Currently we assume that 0 is the default AS, which is usually true, but it isn't for SPIR-V.

Pass down the AS from clang and use it to create types.

After this change, we finally generate fully valid SPIR-V for a basic OpenMP Offloading example.

sarnex · 2025-09-12T14:56:22Z

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

  std::optional<omp::GV> GridValue;

  /// When compilation is being done for the OpenMP host (i.e. `IsTargetDevice =
  /// false`), this contains the list of offloading triples associated, if any.
  SmallVector<Triple> TargetTriples;

+  // Default address space for the target.
+  unsigned DefaultTargetAS = 0;


I didn't use std::optional here because we create OpenMPIRBuilder directly in a ton of unit tests directly and I didn't want to update them all, and also I think that since we already assume the default AS is zero we can continue doing that in cases where we don't set it from clang.

Signed-off-by: Sarnie, Nick <nick.sarnie@intel.com>

llvmbot · 2025-09-12T17:23:57Z

@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-clang-codegen

Author: Nick Sarnie (sarnex)

Changes

Currently we assume that 0 is the default AS, which is usually true, but it isn't for SPIR-V.

Pass down the AS from clang and use it to create types.

After this change, we finally generate fully valid SPIR-V for a basic OpenMP Offloading example.

Full diff: https://github.com/llvm/llvm-project/pull/158152.diff

5 Files Affected:

(modified) clang/lib/CodeGen/CGOpenMPRuntime.cpp (+4-1)
(modified) clang/test/OpenMP/spirv_locstr.cpp (+1-1)
(modified) llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h (+8-2)
(modified) llvm/include/llvm/Frontend/OpenMP/OMPKinds.def (+1-1)
(modified) llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp (+4-3)

diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index e80aa1592f252..a503aaf613e30 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -1037,12 +1037,15 @@ CGOpenMPRuntime::CGOpenMPRuntime(CodeGenModule &CGM)
       CGM.getLangOpts().OpenMPOffloadMandatory,
       /*HasRequiresReverseOffload*/ false, /*HasRequiresUnifiedAddress*/ false,
       hasRequiresUnifiedSharedMemory(), /*HasRequiresDynamicAllocators*/ false);
+  Config.setDefaultTargetAS(
+      CGM.getContext().getTargetInfo().getTargetAddressSpace(LangAS::Default));
+
+  OMPBuilder.setConfig(Config);
   OMPBuilder.initialize();
   OMPBuilder.loadOffloadInfoMetadata(*CGM.getFileSystem(),
                                      CGM.getLangOpts().OpenMPIsTargetDevice
                                          ? CGM.getLangOpts().OMPHostIRFile
                                          : StringRef{});
-  OMPBuilder.setConfig(Config);
 
   // The user forces the compiler to behave as if omp requires
   // unified_shared_memory was given.
diff --git a/clang/test/OpenMP/spirv_locstr.cpp b/clang/test/OpenMP/spirv_locstr.cpp
index 20d9c9d2f7393..80f05029dd8a4 100644
--- a/clang/test/OpenMP/spirv_locstr.cpp
+++ b/clang/test/OpenMP/spirv_locstr.cpp
@@ -4,7 +4,7 @@
 // expected-no-diagnostics
 
 // CHECK: @[[#LOC:]] = private unnamed_addr addrspace(1) constant [23 x i8] c";unknown;unknown;0;0;;\00", align 1
-// CHECK: = private unnamed_addr addrspace(1) constant %struct.ident_t { i32 0, i32 2, i32 0, i32 {{.*}}, ptr addrspacecast (ptr addrspace(1) @[[#LOC]] to ptr) }, align 8
+// CHECK: = private unnamed_addr addrspace(1) constant %struct.ident_t { i32 0, i32 2, i32 0, i32 {{.*}}, ptr addrspace(4) addrspacecast (ptr addrspace(1) @[[#LOC]] to ptr addrspace(4)) }, align 8
 
 int main() {
   int ret = 0;
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
index cc1177ba3d11c..f43ef932e965a 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -125,16 +125,19 @@ class OpenMPIRBuilderConfig {
 
   /// First separator used between the initial two parts of a name.
   std::optional<StringRef> FirstSeparator;
-  /// Separator used between all of the rest consecutive parts of s name
+  /// Separator used between all of the rest consecutive parts of s name.
   std::optional<StringRef> Separator;
 
-  // Grid Value for the GPU target
+  // Grid Value for the GPU target.
   std::optional<omp::GV> GridValue;
 
   /// When compilation is being done for the OpenMP host (i.e. `IsTargetDevice =
   /// false`), this contains the list of offloading triples associated, if any.
   SmallVector<Triple> TargetTriples;
 
+  // Default address space for the target.
+  unsigned DefaultTargetAS = 0;
+
   LLVM_ABI OpenMPIRBuilderConfig();
   LLVM_ABI OpenMPIRBuilderConfig(bool IsTargetDevice, bool IsGPU,
                                  bool OpenMPOffloadMandatory,
@@ -165,6 +168,8 @@ class OpenMPIRBuilderConfig {
     return *GridValue;
   }
 
+  unsigned getDefaultTargetAS() const { return DefaultTargetAS; }
+
   bool hasRequiresFlags() const { return RequiresFlags; }
   LLVM_ABI bool hasRequiresReverseOffload() const;
   LLVM_ABI bool hasRequiresUnifiedAddress() const;
@@ -202,6 +207,7 @@ class OpenMPIRBuilderConfig {
   void setFirstSeparator(StringRef FS) { FirstSeparator = FS; }
   void setSeparator(StringRef S) { Separator = S; }
   void setGridValue(omp::GV G) { GridValue = G; }
+  void setDefaultTargetAS(unsigned AS) { DefaultTargetAS = AS; }
 
   LLVM_ABI void setHasRequiresReverseOffload(bool Value);
   LLVM_ABI void setHasRequiresUnifiedAddress(bool Value);
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPKinds.def b/llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
index 71f041ac138e3..01ca8da759ef7 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -26,7 +26,7 @@
 #endif
 
 #define __OMP_TYPE(VarName) OMP_TYPE(VarName, Type::get##VarName##Ty(Ctx))
-#define __OMP_PTR_TYPE(VarName) OMP_TYPE(VarName, PointerType::get(Ctx, 0))
+#define __OMP_PTR_TYPE(VarName) OMP_TYPE(VarName, PointerType::get(Ctx, DefaultTargetAS))
 
 __OMP_TYPE(Void)
 __OMP_TYPE(Int1)
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index c955ecd403633..d1f78c32596ba 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -10056,19 +10056,20 @@ OpenMPIRBuilder::createOffloadMapnames(SmallVectorImpl<llvm::Constant *> &Names,
 void OpenMPIRBuilder::initializeTypes(Module &M) {
   LLVMContext &Ctx = M.getContext();
   StructType *T;
+  unsigned DefaultTargetAS = Config.getDefaultTargetAS();
 #define OMP_TYPE(VarName, InitValue) VarName = InitValue;
 #define OMP_ARRAY_TYPE(VarName, ElemTy, ArraySize)                             \
   VarName##Ty = ArrayType::get(ElemTy, ArraySize);                             \
-  VarName##PtrTy = PointerType::getUnqual(Ctx);
+  VarName##PtrTy = PointerType::get(Ctx, DefaultTargetAS);
 #define OMP_FUNCTION_TYPE(VarName, IsVarArg, ReturnType, ...)                  \
   VarName = FunctionType::get(ReturnType, {__VA_ARGS__}, IsVarArg);            \
-  VarName##Ptr = PointerType::getUnqual(Ctx);
+  VarName##Ptr = PointerType::get(Ctx, DefaultTargetAS);
 #define OMP_STRUCT_TYPE(VarName, StructName, Packed, ...)                      \
   T = StructType::getTypeByName(Ctx, StructName);                              \
   if (!T)                                                                      \
     T = StructType::create(Ctx, {__VA_ARGS__}, StructName, Packed);            \
   VarName = T;                                                                 \
-  VarName##Ptr = PointerType::getUnqual(Ctx);
+  VarName##Ptr = PointerType::get(Ctx, DefaultTargetAS);
 #include "llvm/Frontend/OpenMP/OMPKinds.def"
 }

llvmbot · 2025-09-12T17:23:58Z

@llvm/pr-subscribers-clang

Author: Nick Sarnie (sarnex)

Changes

Currently we assume that 0 is the default AS, which is usually true, but it isn't for SPIR-V.

Pass down the AS from clang and use it to create types.

After this change, we finally generate fully valid SPIR-V for a basic OpenMP Offloading example.

Full diff: https://github.com/llvm/llvm-project/pull/158152.diff

5 Files Affected:

(modified) clang/lib/CodeGen/CGOpenMPRuntime.cpp (+4-1)
(modified) clang/test/OpenMP/spirv_locstr.cpp (+1-1)
(modified) llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h (+8-2)
(modified) llvm/include/llvm/Frontend/OpenMP/OMPKinds.def (+1-1)
(modified) llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp (+4-3)

diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index e80aa1592f252..a503aaf613e30 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -1037,12 +1037,15 @@ CGOpenMPRuntime::CGOpenMPRuntime(CodeGenModule &CGM)
       CGM.getLangOpts().OpenMPOffloadMandatory,
       /*HasRequiresReverseOffload*/ false, /*HasRequiresUnifiedAddress*/ false,
       hasRequiresUnifiedSharedMemory(), /*HasRequiresDynamicAllocators*/ false);
+  Config.setDefaultTargetAS(
+      CGM.getContext().getTargetInfo().getTargetAddressSpace(LangAS::Default));
+
+  OMPBuilder.setConfig(Config);
   OMPBuilder.initialize();
   OMPBuilder.loadOffloadInfoMetadata(*CGM.getFileSystem(),
                                      CGM.getLangOpts().OpenMPIsTargetDevice
                                          ? CGM.getLangOpts().OMPHostIRFile
                                          : StringRef{});
-  OMPBuilder.setConfig(Config);
 
   // The user forces the compiler to behave as if omp requires
   // unified_shared_memory was given.
diff --git a/clang/test/OpenMP/spirv_locstr.cpp b/clang/test/OpenMP/spirv_locstr.cpp
index 20d9c9d2f7393..80f05029dd8a4 100644
--- a/clang/test/OpenMP/spirv_locstr.cpp
+++ b/clang/test/OpenMP/spirv_locstr.cpp
@@ -4,7 +4,7 @@
 // expected-no-diagnostics
 
 // CHECK: @[[#LOC:]] = private unnamed_addr addrspace(1) constant [23 x i8] c";unknown;unknown;0;0;;\00", align 1
-// CHECK: = private unnamed_addr addrspace(1) constant %struct.ident_t { i32 0, i32 2, i32 0, i32 {{.*}}, ptr addrspacecast (ptr addrspace(1) @[[#LOC]] to ptr) }, align 8
+// CHECK: = private unnamed_addr addrspace(1) constant %struct.ident_t { i32 0, i32 2, i32 0, i32 {{.*}}, ptr addrspace(4) addrspacecast (ptr addrspace(1) @[[#LOC]] to ptr addrspace(4)) }, align 8
 
 int main() {
   int ret = 0;
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
index cc1177ba3d11c..f43ef932e965a 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -125,16 +125,19 @@ class OpenMPIRBuilderConfig {
 
   /// First separator used between the initial two parts of a name.
   std::optional<StringRef> FirstSeparator;
-  /// Separator used between all of the rest consecutive parts of s name
+  /// Separator used between all of the rest consecutive parts of s name.
   std::optional<StringRef> Separator;
 
-  // Grid Value for the GPU target
+  // Grid Value for the GPU target.
   std::optional<omp::GV> GridValue;
 
   /// When compilation is being done for the OpenMP host (i.e. `IsTargetDevice =
   /// false`), this contains the list of offloading triples associated, if any.
   SmallVector<Triple> TargetTriples;
 
+  // Default address space for the target.
+  unsigned DefaultTargetAS = 0;
+
   LLVM_ABI OpenMPIRBuilderConfig();
   LLVM_ABI OpenMPIRBuilderConfig(bool IsTargetDevice, bool IsGPU,
                                  bool OpenMPOffloadMandatory,
@@ -165,6 +168,8 @@ class OpenMPIRBuilderConfig {
     return *GridValue;
   }
 
+  unsigned getDefaultTargetAS() const { return DefaultTargetAS; }
+
   bool hasRequiresFlags() const { return RequiresFlags; }
   LLVM_ABI bool hasRequiresReverseOffload() const;
   LLVM_ABI bool hasRequiresUnifiedAddress() const;
@@ -202,6 +207,7 @@ class OpenMPIRBuilderConfig {
   void setFirstSeparator(StringRef FS) { FirstSeparator = FS; }
   void setSeparator(StringRef S) { Separator = S; }
   void setGridValue(omp::GV G) { GridValue = G; }
+  void setDefaultTargetAS(unsigned AS) { DefaultTargetAS = AS; }
 
   LLVM_ABI void setHasRequiresReverseOffload(bool Value);
   LLVM_ABI void setHasRequiresUnifiedAddress(bool Value);
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPKinds.def b/llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
index 71f041ac138e3..01ca8da759ef7 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -26,7 +26,7 @@
 #endif
 
 #define __OMP_TYPE(VarName) OMP_TYPE(VarName, Type::get##VarName##Ty(Ctx))
-#define __OMP_PTR_TYPE(VarName) OMP_TYPE(VarName, PointerType::get(Ctx, 0))
+#define __OMP_PTR_TYPE(VarName) OMP_TYPE(VarName, PointerType::get(Ctx, DefaultTargetAS))
 
 __OMP_TYPE(Void)
 __OMP_TYPE(Int1)
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index c955ecd403633..d1f78c32596ba 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -10056,19 +10056,20 @@ OpenMPIRBuilder::createOffloadMapnames(SmallVectorImpl<llvm::Constant *> &Names,
 void OpenMPIRBuilder::initializeTypes(Module &M) {
   LLVMContext &Ctx = M.getContext();
   StructType *T;
+  unsigned DefaultTargetAS = Config.getDefaultTargetAS();
 #define OMP_TYPE(VarName, InitValue) VarName = InitValue;
 #define OMP_ARRAY_TYPE(VarName, ElemTy, ArraySize)                             \
   VarName##Ty = ArrayType::get(ElemTy, ArraySize);                             \
-  VarName##PtrTy = PointerType::getUnqual(Ctx);
+  VarName##PtrTy = PointerType::get(Ctx, DefaultTargetAS);
 #define OMP_FUNCTION_TYPE(VarName, IsVarArg, ReturnType, ...)                  \
   VarName = FunctionType::get(ReturnType, {__VA_ARGS__}, IsVarArg);            \
-  VarName##Ptr = PointerType::getUnqual(Ctx);
+  VarName##Ptr = PointerType::get(Ctx, DefaultTargetAS);
 #define OMP_STRUCT_TYPE(VarName, StructName, Packed, ...)                      \
   T = StructType::getTypeByName(Ctx, StructName);                              \
   if (!T)                                                                      \
     T = StructType::create(Ctx, {__VA_ARGS__}, StructName, Packed);            \
   VarName = T;                                                                 \
-  VarName##Ptr = PointerType::getUnqual(Ctx);
+  VarName##Ptr = PointerType::get(Ctx, DefaultTargetAS);
 #include "llvm/Frontend/OpenMP/OMPKinds.def"
 }

tblah

The changes LGTM thanks.

if you are trying to enable SPIR-V more generally, you will also need to set this in mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp to fix the other upstream user of OMPIRBuilder. There are already code examples in that file showing how to fetch the default address space from the data layout string in the MILR module, but please do ping me if I can help.

sarnex · 2025-09-15T14:24:07Z

Thanks Tom! In my case I'm never going through mlir yet, but I should make it work, so I will make a separate PR quickly after this to fix MLIR and add you as a reviewer.

Extension of #158152 for MLIR. --------- Signed-off-by: Sarnie, Nick <nick.sarnie@intel.com>

…er (#158689) Extension of llvm/llvm-project#158152 for MLIR. --------- Signed-off-by: Sarnie, Nick <nick.sarnie@intel.com>

sarnex commented Sep 12, 2025

View reviewed changes

[clang][OMPIRBuilder] Use default target AS to create types

6ccd593

Signed-off-by: Sarnie, Nick <nick.sarnie@intel.com>

sarnex force-pushed the types branch from 2f954b7 to 6ccd593 Compare September 12, 2025 14:58

sarnex marked this pull request as ready for review September 12, 2025 17:23

llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. flang:openmp clang:openmp OpenMP related changes to Clang labels Sep 12, 2025

sarnex requested review from tblah, abidh and jhuber6 September 12, 2025 17:25

tblah approved these changes Sep 15, 2025

View reviewed changes

Merge remote-tracking branch 'upstream/main'

546124e

sarnex merged commit c7acd07 into llvm:main Sep 15, 2025
11 checks passed

sarnex mentioned this pull request Sep 15, 2025

[MLIR][OpenMP] Set default address space for OpenMPIRBuilder #158689

Merged

sarnex added a commit that referenced this pull request Sep 16, 2025

[MLIR][OpenMP] Set default address space for OpenMPIRBuilder (#158689)

148e099

Extension of #158152 for MLIR. --------- Signed-off-by: Sarnie, Nick <nick.sarnie@intel.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[clang][OMPIRBuilder] Use default target AS to create types #158152

[clang][OMPIRBuilder] Use default target AS to create types #158152

sarnex commented Sep 11, 2025 •

edited

Loading

Uh oh!

sarnex Sep 12, 2025

Uh oh!

llvmbot commented Sep 12, 2025 •

edited

Loading

Uh oh!

llvmbot commented Sep 12, 2025

Uh oh!

tblah left a comment •

edited

Loading

Uh oh!

sarnex commented Sep 15, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

[clang][OMPIRBuilder] Use default target AS to create types #158152

[clang][OMPIRBuilder] Use default target AS to create types #158152

Conversation

sarnex commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sarnex Sep 12, 2025

Choose a reason for hiding this comment

Uh oh!

llvmbot commented Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Sep 12, 2025

Uh oh!

tblah left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sarnex commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sarnex commented Sep 11, 2025 •

edited

Loading

llvmbot commented Sep 12, 2025 •

edited

Loading

tblah left a comment •

edited

Loading

sarnex commented Sep 15, 2025 •

edited

Loading