[SYCL] Add aspect enum value information to LLVM IR metadata (#6804)

steffenlarsen · web-flow · commit 4d0df2226bf0 · 2022-09-23T14:26:04.000+01:00
This commit adds a new module-level metadata node !intel_sycl_aspects
which contains metadata pairs of the enum element names and integral
values of the SYCL aspects enum, identified by the
[[__sycl_detail__::sycl_type(aspect)]] attribute.
This commit also makes the SYCLPropagateAspectsUsage pass read
!intel_sycl_aspects and use this information to determine the value of
the fp64 aspect instead of relying on it being synchronized with the
SYCL implementation headers.

Signed-off-by: Larsen, Steffen &lt;steffen.larsen@intel.com&gt;
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -527,6 +527,17 @@ static llvm::MDNode *getAspectsMD(ASTContext &ASTContext,
   return llvm::MDNode::get(Ctx, AspectsMD);
 }
 
+static llvm::MDNode *getAspectEnumValueMD(ASTContext &ASTContext,
+                                          llvm::LLVMContext &Ctx,
+                                          const EnumConstantDecl *ECD) {
+  SmallVector<llvm::Metadata *, 2> AspectEnumValMD;
+  AspectEnumValMD.push_back(llvm::MDString::get(Ctx, ECD->getName()));
+  AspectEnumValMD.push_back(
+      llvm::ConstantAsMetadata::get(llvm::ConstantInt::get(
+          llvm::Type::getInt32Ty(Ctx), ECD->getInitVal().getSExtValue())));
+  return llvm::MDNode::get(Ctx, AspectEnumValMD);
+}
+
 void CodeGenModule::Release() {
   Module *Primary = getContext().getModuleForCodeGen();
   if (CXX20ModuleInits && Primary && !Primary->isHeaderLikeModule())
@@ -924,6 +935,15 @@ void CodeGenModule::Release() {
                                            RD->getAttr<SYCLUsesAspectsAttr>()));
       }
     }
+
+    // Emit metadata for all aspects defined in the aspects enum.
+    if (AspectsEnumDecl) {
+      llvm::NamedMDNode *AspectEnumValsMD =
+          TheModule.getOrInsertNamedMetadata("sycl_aspects");
+      for (const EnumConstantDecl *ECD : AspectsEnumDecl->enumerators())
+        AspectEnumValsMD->addOperand(
+            getAspectEnumValueMD(Context, TheModule.getContext(), ECD));
+    }
   }
 
   // HLSL related end of code gen work items.
@@ -5063,6 +5083,16 @@ void CodeGenModule::maybeSetTrivialComdat(const Decl &D,
   GO.setComdat(TheModule.getOrInsertComdat(GO.getName()));
 }
 
+void CodeGenModule::setAspectsEnumDecl(const EnumDecl *ED) {
+  if (AspectsEnumDecl && AspectsEnumDecl != ED) {
+    // Conflicting definitions of the aspect enum are not allowed.
+    Error(ED->getLocation(), "redefinition of aspect enum");
+    getDiags().Report(AspectsEnumDecl->getLocation(),
+                      diag::note_previous_definition);
+  }
+  AspectsEnumDecl = ED;
+}
+
 void CodeGenModule::generateIntelFPGAAnnotation(
     const Decl *D, llvm::SmallString<256> &AnnotStr) {
   llvm::raw_svector_ostream Out(AnnotStr);
diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h
@@ -604,6 +604,7 @@ class CodeGenModule : public CodeGenTypeCache {
   llvm::DenseMap<const llvm::Constant *, llvm::GlobalVariable *> RTTIProxyMap;
 
   llvm::DenseMap<StringRef, const RecordDecl *> TypesWithAspects;
+  const EnumDecl *AspectsEnumDecl = nullptr;
 
 public:
   CodeGenModule(ASTContext &C, IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS,
@@ -1098,6 +1099,8 @@ class CodeGenModule : public CodeGenTypeCache {
     TypesWithAspects[TypeName] = RD;
   }
 
+  void setAspectsEnumDecl(const EnumDecl *ED);
+
   void generateIntelFPGAAnnotation(const Decl *D,
                                      llvm::SmallString<256> &AnnotStr);
   void addGlobalIntelFPGAAnnotation(const VarDecl *VD, llvm::GlobalValue *GV);
diff --git a/clang/lib/CodeGen/CodeGenTypes.cpp b/clang/lib/CodeGen/CodeGenTypes.cpp
@@ -321,6 +321,10 @@ void CodeGenTypes::UpdateCompletedType(const TagDecl *TD) {
       if (!ConvertType(ED->getIntegerType())->isIntegerTy(32))
         TypeCache.clear();
     }
+    // If this is the SYCL aspect enum it is saved for later processing.
+    if (const auto *Attr = ED->getAttr<SYCLTypeAttr>())
+      if (Attr->getType() == SYCLTypeAttr::SYCLType::aspect)
+        CGM.setAspectsEnumDecl(ED);
     // If necessary, provide the full definition of a type only used with a
     // declaration so far.
     if (CGDebugInfo *DI = CGM.getModuleDebugInfo())
diff --git a/clang/test/CodeGenSYCL/Inputs/sycl.hpp b/clang/test/CodeGenSYCL/Inputs/sycl.hpp
@@ -70,7 +70,7 @@ enum class address_space : int {
 } // namespace access
 
 // Dummy aspect enum with limited enumerators
-enum class __SYCL_TYPE(aspect) aspect {
+enum class __SYCL_TYPE(aspect) aspect { // #AspectEnum
   host = 0,
   cpu = 1,
   gpu = 2,
diff --git a/clang/test/CodeGenSYCL/aspect_enum.cpp b/clang/test/CodeGenSYCL/aspect_enum.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -internal-isystem %S/Inputs -fsycl-is-device -triple spir64-unknown-unknown -disable-llvm-passes -emit-llvm %s -o - | FileCheck %s
+
+// Tests for IR of [[__sycl_detail__::sycl_type(aspect)]] enum.
+#include "sycl.hpp"
+
+// CHECK: !sycl_aspects = !{![[HOST:[0-9]+]], ![[CPU:[0-9]+]], ![[GPU:[0-9]+]], ![[ACC:[0-9]+]], ![[CUSTOM:[0-9]+]], ![[FP16:[0-9]+]], ![[FP64:[0-9]+]]}
+// CHECK: ![[HOST]] = !{!"host", i32 0}
+// CHECK: ![[CPU]] = !{!"cpu", i32 1}
+// CHECK: ![[GPU]] = !{!"gpu", i32 2}
+// CHECK: ![[ACC]] = !{!"accelerator", i32 3}
+// CHECK: ![[CUSTOM]] = !{!"custom", i32 4}
+// CHECK: ![[FP16]] = !{!"fp16", i32 5}
+// CHECK: ![[FP64]] = !{!"fp64", i32 6}
diff --git a/clang/test/CodeGenSYCL/multiple_aspect_enum.cpp b/clang/test/CodeGenSYCL/multiple_aspect_enum.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -internal-isystem %S/Inputs -fsycl-is-device -triple spir64-unknown-unknown -verify -emit-llvm-only %s
+
+// Tests for error diagnostics when multiple definitions of
+// [[__sycl_detail__::sycl_type(aspect)]] enums are present.
+#include "sycl.hpp"
+
+// expected-note@#AspectEnum{{previous definition is here}}
+
+// expected-error@+1{{redefinition of aspect enum}}
+enum class [[__sycl_detail__::sycl_type(aspect)]] aspect_redef {
+  imposter_value = 3
+};
diff --git a/llvm/lib/SYCLLowerIR/SYCLPropagateAspectsUsage.cpp b/llvm/lib/SYCLLowerIR/SYCLPropagateAspectsUsage.cpp
@@ -79,6 +79,35 @@ TypeToAspectsMapTy getTypesThatUseAspectsFromMetadata(const Module &M) {
   return Result;
 }
 
+using AspectValueToNameMapTy = SmallMapVector<StringRef, int, 32>;
+
+/// Retrieves from metadata (sycl_aspects) the mapping between SYCL aspect names
+/// and their integral values.
+AspectValueToNameMapTy getAspectsFromMetadata(const Module &M) {
+  const NamedMDNode *Node = M.getNamedMetadata("sycl_aspects");
+  AspectValueToNameMapTy Result;
+  if (!Node)
+    return Result;
+
+  for (const auto OperandIt : Node->operands()) {
+    const MDNode &N = *OperandIt;
+    assert(N.getNumOperands() == 2 &&
+           "Each operand of sycl_aspects must be a pair.");
+
+    // The aspect's name is the first operand.
+    const auto *AspectName = cast<MDString>(N.getOperand(0));
+
+    // The aspect's integral value is the second operand.
+    const auto *AspectCAM = cast<ConstantAsMetadata>(N.getOperand(1));
+    const Constant *AspectC = AspectCAM->getValue();
+
+    Result[AspectName->getString()] =
+        cast<ConstantInt>(AspectC)->getSExtValue();
+  }
+
+  return Result;
+}
+
 using TypesEdgesTy =
     std::unordered_map<const Type *, std::vector<const Type *>>;
 
@@ -107,6 +136,7 @@ void propagateAspectsThroughTypes(const TypesEdgesTy &Edges, const Type *Start,
 /// another type TT, which in turn uses the aspect A.
 /// @TypesWithAspects argument consist of known types with aspects
 /// from metadata information.
+/// @AspectValues argument consist of known aspect values and their names.
 ///
 /// The algorithm is the following:
 /// 1) Make a list of all structure types from module @M. The list also
@@ -121,18 +151,17 @@ void propagateAspectsThroughTypes(const TypesEdgesTy &Edges, const Type *Start,
 /// Time complexity: O((V + E) * T) where T is the number of input types
 /// containing aspects.
 void propagateAspectsToOtherTypesInModule(
-    const Module &M, TypeToAspectsMapTy &TypesWithAspects) {
+    const Module &M, TypeToAspectsMapTy &TypesWithAspects,
+    AspectValueToNameMapTy &AspectValues) {
   std::unordered_set<const Type *> TypesToProcess;
   const Type *DoubleTy = Type::getDoubleTy(M.getContext());
 
-  // 6 is taken from sycl/include/CL/sycl/aspects.hpp
-  // Note: that magic number must strictly correspond to the one assigned to
-  // 'fp64' value of 'aspect' enum.
-  // FIXME: we should develop some kind of mechanism which will allow us to
-  // avoid hardcoding this number here and having a build dependency between
-  // the compiler and the runtime. See intel/llvm#5892
-  static constexpr int AspectFP64 = 6;
-  TypesWithAspects[DoubleTy].insert(AspectFP64);
+  // Find the value of the fp64 aspect from the aspect values map and register
+  // it as a special-case type with aspect for double.
+  auto FP64AspectIt = AspectValues.find("fp64");
+  assert(FP64AspectIt != AspectValues.end() &&
+         "fp64 aspect was not found in the aspect values.");
+  TypesWithAspects[DoubleTy].insert(FP64AspectIt->second);
 
   TypesToProcess.insert(DoubleTy);
   for (const Type *T : M.getIdentifiedStructTypes())
@@ -339,7 +368,19 @@ buildFunctionsToAspectsMap(Module &M, TypeToAspectsMapTy &TypesWithAspects) {
 PreservedAnalyses
 SYCLPropagateAspectsUsagePass::run(Module &M, ModuleAnalysisManager &MAM) {
   TypeToAspectsMapTy TypesWithAspects = getTypesThatUseAspectsFromMetadata(M);
-  propagateAspectsToOtherTypesInModule(M, TypesWithAspects);
+  AspectValueToNameMapTy AspectValues = getAspectsFromMetadata(M);
+
+  // If there is no metadata for aspect values the source code must not have
+  // included the SYCL headers. In that case there should also not be any types
+  // that use aspects, so we can skip this pass.
+  if (AspectValues.empty()) {
+    assert(TypesWithAspects.empty() &&
+           "sycl_aspects metadata is missing but "
+           "sycl_types_that_use_aspects is present.");
+    return PreservedAnalyses::all();
+  }
+
+  propagateAspectsToOtherTypesInModule(M, TypesWithAspects, AspectValues);
 
   FunctionToAspectsMapTy FunctionToAspects =
       buildFunctionsToAspectsMap(M, TypesWithAspects);
diff --git a/llvm/test/SYCLLowerIR/PropagateAspectsUsage/call-graph-1.ll b/llvm/test/SYCLLowerIR/PropagateAspectsUsage/call-graph-1.ll
@@ -50,6 +50,9 @@ define spir_func void @func3() {
 !0 = !{!"Optional.A", i32 1}
 !1 = !{!"Optional.B", i32 2}
 
+!sycl_aspects = !{!2}
+!2 = !{!"fp64", i32 6}
+
 ; CHECK: ![[#ID1]] = !{i32 1}
 ; CHECK: ![[#ID2]] = !{i32 1, i32 2}
 ; CHECK: ![[#ID3]] = !{i32 2}
diff --git a/llvm/test/SYCLLowerIR/PropagateAspectsUsage/call-graph-2.ll b/llvm/test/SYCLLowerIR/PropagateAspectsUsage/call-graph-2.ll
@@ -51,6 +51,9 @@ define spir_func void @func4() {
 !0 = !{!"Optional.A", i32 1}
 !1 = !{!"Optional.B", i32 2}
 
+!sycl_aspects = !{!2}
+!2 = !{!"fp64", i32 6}
+
 ; CHECK: ![[#ID1]] = !{i32 1, i32 2}
 ; CHECK: ![[#ID2]] = !{i32 1}
 ; CHECK: ![[#ID3]] = !{i32 2}
diff --git a/llvm/test/SYCLLowerIR/PropagateAspectsUsage/composite-types-1.ll b/llvm/test/SYCLLowerIR/PropagateAspectsUsage/composite-types-1.ll
@@ -22,13 +22,13 @@
 
 %F2.does.not.contain.optional = type { %B.core, %C.core*, %D2.does.not.contain.optional* }
 
-; CHECK: spir_kernel void @kernelD1.uses.optional() !sycl_used_aspects !1 {
+; CHECK: spir_kernel void @kernelD1.uses.optional() !sycl_used_aspects ![[MDID:[0-9]+]] {
 define spir_kernel void @kernelD1.uses.optional() {
   %tmp = alloca %D1.contains.optional
   ret void
 }
 
-; CHECK: spir_func void @funcD1.uses.optional() !sycl_used_aspects !1 {
+; CHECK: spir_func void @funcD1.uses.optional() !sycl_used_aspects ![[MDID]] {
 define spir_func void @funcD1.uses.optional() {
   %tmp = alloca %D1.contains.optional
   ret void
@@ -46,13 +46,13 @@ define spir_func void @funcD2.does.not.use.optional() {
   ret void
 }
 
-; CHECK: spir_kernel void @kernelE.uses.optional() !sycl_used_aspects !1 {
+; CHECK: spir_kernel void @kernelE.uses.optional() !sycl_used_aspects ![[MDID]] {
 define spir_kernel void @kernelE.uses.optional() {
   %tmp = alloca %E.contains.optional
   ret void
 }
 
-; CHECK: spir_func void @funcE.uses.optional() !sycl_used_aspects !1 {
+; CHECK: spir_func void @funcE.uses.optional() !sycl_used_aspects ![[MDID]] {
 define spir_func void @funcE.uses.optional() {
   %tmp = alloca %E.contains.optional
   ret void
@@ -82,25 +82,28 @@ define spir_func void @funcF2.does.not.use.optional() {
   ret void
 }
 
-; CHECK: spir_func %A.optional @funcA.returns.optional() !sycl_used_aspects !1 {
+; CHECK: spir_func %A.optional @funcA.returns.optional() !sycl_used_aspects ![[MDID]] {
 define spir_func %A.optional @funcA.returns.optional() {
   %tmp = alloca %A.optional
   %ret = load %A.optional, %A.optional* %tmp
   ret %A.optional %ret
 }
 
-; CHECK: spir_func void @funcA.uses.array.of.optional() !sycl_used_aspects !1 {
+; CHECK: spir_func void @funcA.uses.array.of.optional() !sycl_used_aspects ![[MDID]] {
 define spir_func void @funcA.uses.array.of.optional() {
   %tmp = alloca [4 x %A.optional]
   ret void
 }
 
-; CHECK: spir_func void @funcA.assepts.optional(%A.optional %0) !sycl_used_aspects !1 {
+; CHECK: spir_func void @funcA.assepts.optional(%A.optional %0) !sycl_used_aspects ![[MDID]] {
 define spir_func void @funcA.assepts.optional(%A.optional %0) {
   ret void
 }
 
 !sycl_types_that_use_aspects = !{!0}
 !0 = !{!"A.optional", i32 1}
 
-; CHECK: !1 = !{i32 1}
+!sycl_aspects = !{!1}
+!1 = !{!"fp64", i32 6}
+
+; CHECK: ![[MDID]] = !{i32 1}
diff --git a/llvm/test/SYCLLowerIR/PropagateAspectsUsage/double.ll b/llvm/test/SYCLLowerIR/PropagateAspectsUsage/double.ll
@@ -4,34 +4,37 @@
 
 %composite = type { double }
 
-; CHECK: spir_kernel void @kernel() !sycl_used_aspects !0 {
+; CHECK: spir_kernel void @kernel() !sycl_used_aspects ![[MDID:[0-9]+]] {
 define spir_kernel void @kernel() {
   call spir_func void @func()
   ret void
 }
 
-; CHECK: spir_func void @func() !sycl_used_aspects !0 {
+; CHECK: spir_func void @func() !sycl_used_aspects ![[MDID]] {
 define spir_func void @func() {
   %tmp = alloca double
   ret void
 }
 
-; CHECK: spir_func void @func.array() !sycl_used_aspects !0 {
+; CHECK: spir_func void @func.array() !sycl_used_aspects ![[MDID]] {
 define spir_func void @func.array() {
   %tmp = alloca [4 x double]
   ret void
 }
 
-; CHECK: spir_func void @func.vector() !sycl_used_aspects !0 {
+; CHECK: spir_func void @func.vector() !sycl_used_aspects ![[MDID]] {
 define spir_func void @func.vector() {
   %tmp = alloca <4 x double>
   ret void
 }
 
-; CHECK: spir_func void @func.composite() !sycl_used_aspects !0 {
+; CHECK: spir_func void @func.composite() !sycl_used_aspects ![[MDID]] {
 define spir_func void @func.composite() {
   %tmp = alloca %composite
   ret void
 }
 
-; CHECK: !0 = !{i32 6}
+!sycl_aspects = !{!0}
+!0 = !{!"fp64", i32 6}
+
+; CHECK: ![[MDID]] = !{i32 6}
diff --git a/llvm/test/SYCLLowerIR/PropagateAspectsUsage/multiple-aspects.ll b/llvm/test/SYCLLowerIR/PropagateAspectsUsage/multiple-aspects.ll
@@ -46,6 +46,9 @@ define spir_kernel void @kernel() {
 !2 = !{!"C", i32 2}
 !3 = !{!"D", i32 3, i32 4}
 
+!sycl_aspects = !{!4}
+!4 = !{!"fp64", i32 6}
+
 ; CHECK: ![[#ID0]] = !{i32 0}
 ; CHECK: ![[#ID1]] = !{i32 1, i32 0}
 ; CHECK: ![[#ID2]] = !{i32 2, i32 1, i32 0}
diff --git a/llvm/test/SYCLLowerIR/PropagateAspectsUsage/no-uses-of-optional.ll b/llvm/test/SYCLLowerIR/PropagateAspectsUsage/no-uses-of-optional.ll
@@ -18,3 +18,6 @@ define weak dso_local spir_func void @func() {
 
 !sycl_types_that_use_aspects = !{!0}
 !0 = !{!"MyStruct", i32 1}
+
+!sycl_aspects = !{!2}
+!2 = !{!"fp64", i32 6}
diff --git a/sycl/doc/design/OptionalDeviceFeatures.md b/sycl/doc/design/OptionalDeviceFeatures.md
@@ -423,6 +423,24 @@ allow metadata to be attached directly to types.  This representation works
 around that limitation by creating global named metadata that references the
 type's name.
 
+To synchronize the integral values of given aspects between the SYCL headers and
+the compiler, the `!sycl_aspects` metadata is added to the module, based on the
+values defined in the enum. Inside this metadata node, each value of the aspect
+enum is represented by another metadata node with two operands; the name of the
+value and the corresponding integral value. An example of this is:
+
+```
+!sycl_aspects = !{!0, !1, !2, ...}
+!0 = !{!"host", i32 0}
+!1 = !{!"cpu", i32 1}
+!2 = !{!"gpu", i32 2}
+...
+```
+
+**NOTE**: The `!sycl_aspects` metadata is both used by the compiler to identify
+the aspect values of implicit aspect requirements, such as `aspect::fp64` from
+the use of `double`, and to offer better diagnostic messages.
+
 We also introduce three metadata that can be attached to a function definition:
 
 * The `!sycl_declared_aspects` metadata is used for functions that are
@@ -483,6 +501,12 @@ to the following rules:
   an `!sycl_declared_aspects` metadata to the function's definition listing
   the aspects from that attribute.
 
+* If a completed enum is decorated with `[[sycl_detail::sycl_type(aspect)]]` the
+  front-end adds an `!sycl_aspects` metadata to the module containing one
+  metadata node for each value in the enum. If there are multiple enum
+  definitions with the `[[sycl_detail::sycl_type(aspect)]]` attribute a
+  diagnostic is issued.
+
 
 ### New LLVM IR pass to propagate aspect usage
 
