Address review comments

Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>

Author: steffenlarsen

clang/lib/CodeGen/CodeGenModule.cpp

@@ -939,7 +939,7 @@ void CodeGenModule::Release() {
     // Emit metadata for all aspects defined in the aspects enum.
     if (AspectsEnumDecl) {
       llvm::NamedMDNode *AspectEnumValsMD =
-          TheModule.getOrInsertNamedMetadata("intel_sycl_aspects");
+          TheModule.getOrInsertNamedMetadata("sycl_aspects");
       for (const EnumConstantDecl *ECD : AspectsEnumDecl->enumerators())
         AspectEnumValsMD->addOperand(
             getAspectEnumValueMD(Context, TheModule.getContext(), ECD));

clang/test/CodeGenSYCL/aspect_enum.cpp

@@ -1,23 +1,13 @@
-// RUN: %clang_cc1 -fsycl-is-device -triple spir64-unknown-unknown -disable-llvm-passes -emit-llvm %s -o - | FileCheck %s
+// 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.
-enum class [[__sycl_detail__::sycl_type(aspect)]] aspect {
-  host = 0,
-  cpu = 1,
-  gpu = 2,
-  accelerator = 3,
-  custom = 4,
-  fp16 = 5,
-  fp64 = 6,
-  future_aspect = 12
-};
+#include "sycl.hpp"
 
-// CHECK: !intel_sycl_aspects = !{![[HOST:[0-9]+]], ![[CPU:[0-9]+]], ![[GPU:[0-9]+]], ![[ACC:[0-9]+]], ![[CUSTOM:[0-9]+]], ![[FP16:[0-9]+]], ![[FP64:[0-9]+]], ![[FUTURE_ASPECT:[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}
-// CHECK: [[FUTURE_ASPECT]] = !{!"future_aspect", i32 12}
+// 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}

llvm/lib/SYCLLowerIR/SYCLPropagateAspectsUsage.cpp

@@ -81,18 +81,18 @@ TypeToAspectsMapTy getTypesThatUseAspectsFromMetadata(const Module &M) {
 
 using AspectValueToNameMapTy = SmallMapVector<StringRef, int, 32>;
 
-/// Retrieves from metadata (intel_sycl_aspects) the mapping between SYCL aspect
-/// names and their integral values.
+/// Retrieves from metadata (sycl_aspects) the mapping between SYCL aspect names
+/// and their integral values.
 AspectValueToNameMapTy getSYCLAspectsFromMetadata(const Module &M) {
-  const NamedMDNode *Node = M.getNamedMetadata("intel_sycl_aspects");
+  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 intel_sycl_aspects must be a pair.");
+           "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));
@@ -369,7 +369,7 @@ SYCLPropagateAspectsUsagePass::run(Module &M, ModuleAnalysisManager &MAM) {
   // that use aspects, so we can skip this pass.
   if (AspectValues.empty()) {
     assert(TypesWithAspects.empty() &&
-           "intel_sycl_aspects metadata is missing but "
+           "sycl_aspects metadata is missing but "
            "intel_types_that_use_aspects is present.");
     return PreservedAnalyses::all();
   }

llvm/test/SYCLLowerIR/PropagateAspectsUsage/call-graph-1.ll

@@ -50,7 +50,7 @@ define spir_func void @func3() {
 !0 = !{!"Optional.A", i32 1}
 !1 = !{!"Optional.B", i32 2}
 
-!intel_sycl_aspects = !{!2}
+!sycl_aspects = !{!2}
 !2 = !{!"fp64", i32 6}
 
 ; CHECK: ![[#ID1]] = !{i32 1}

llvm/test/SYCLLowerIR/PropagateAspectsUsage/call-graph-2.ll

@@ -51,7 +51,7 @@ define spir_func void @func4() {
 !0 = !{!"Optional.A", i32 1}
 !1 = !{!"Optional.B", i32 2}
 
-!intel_sycl_aspects = !{!2}
+!sycl_aspects = !{!2}
 !2 = !{!"fp64", i32 6}
 
 ; CHECK: ![[#ID1]] = !{i32 1, i32 2}

llvm/test/SYCLLowerIR/PropagateAspectsUsage/composite-types-1.ll

@@ -103,7 +103,7 @@ define spir_func void @funcA.assepts.optional(%A.optional %0) {
 !intel_types_that_use_aspects = !{!0}
 !0 = !{!"A.optional", i32 1}
 
-!intel_sycl_aspects = !{!1}
+!sycl_aspects = !{!1}
 !1 = !{!"fp64", i32 6}
 
 ; CHECK: ![[MDID]] = !{i32 1}

llvm/test/SYCLLowerIR/PropagateAspectsUsage/double.ll

@@ -34,7 +34,7 @@ define spir_func void @func.composite() {
   ret void
 }
 
-!intel_sycl_aspects = !{!0}
+!sycl_aspects = !{!0}
 !0 = !{!"fp64", i32 6}
 
 ; CHECK: ![[MDID]] = !{i32 6}

llvm/test/SYCLLowerIR/PropagateAspectsUsage/multiple-aspects.ll

@@ -46,7 +46,7 @@ define spir_kernel void @kernel() {
 !2 = !{!"C", i32 2}
 !3 = !{!"D", i32 3, i32 4}
 
-!intel_sycl_aspects = !{!4}
+!sycl_aspects = !{!4}
 !4 = !{!"fp64", i32 6}
 
 ; CHECK: ![[#ID0]] = !{i32 0}

llvm/test/SYCLLowerIR/PropagateAspectsUsage/no-uses-of-optional.ll

@@ -19,5 +19,5 @@ define weak dso_local spir_func void @func() {
 !intel_types_that_use_aspects = !{!0}
 !0 = !{!"MyStruct", i32 1}
 
-!intel_sycl_aspects = !{!2}
+!sycl_aspects = !{!2}
 !2 = !{!"fp64", i32 6}

sycl/doc/design/OptionalDeviceFeatures.md

@@ -424,19 +424,23 @@ 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 `!intel_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:
+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:
 
 ```
-!intel_sycl_aspects = !{!0, !1, !2, ...}
+!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
 similar to the existing `!intel_reqd_sub_group_size`:
 
@@ -499,7 +503,7 @@ to the following rules:
   the aspects from that attribute.
 
 * If a completed enum is decorated with `[[sycl_detail::sycl_type(aspect)]]` the
-  front-end adds an `!intel_sycl_aspects` metadata to the module containing one
+  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.