[PATCH] D17764: Add attributes for AMD GPU Tools

[Matt Arsenault via cfe-commits]

arsenm added inline comments.

================
Comment at: include/clang/Basic/Attr.td:993-998
@@ +992,8 @@
+
+def AMDGPUToolsInsertNops : InheritableAttr {
+  let Spellings = [GNU<"amdgpu_tools_insert_nops">];
+  let Documentation = [AMDGPUToolsInsertNopsDocs];
+  let Subjects = SubjectList<[Function], ErrorDiag,
+                             "ExpectedKernelFunction">;
+}
+
----------------
I didn't envision these as being user code facing attributes, and only the function emission would add them. Is there a use for these being used manually?

================
Comment at: include/clang/Driver/Options.td:355-365
@@ -354,2 +354,13 @@
 def allowable__client : Separate<["-"], "allowable_client">;
+def amdgpu_tools_insert_nops :
+  Flag<["--"], "amdgpu-tools-insert-nops">, Flags<[CC1Option, HelpHidden]>,
+  HelpText<"Insert two nop instructions for each high level source statement">;
+def amdgpu_tools_num_reserved_vgpr :
+  Joined<["--"], "amdgpu-tools-num-reserved-vgpr=">,
+  Flags<[CC1Option, HelpHidden]>,
+  HelpText<"Reserve <num> vector registers">, MetaVarName<"<num>">;
+def amdgpu_tools_num_reserved_sgpr :
+  Joined<["--"], "amdgpu-tools-num-reserved-sgpr=">,
+  Flags<[CC1Option, HelpHidden]>,
+  HelpText<"Reserve <num> scalar registers">, MetaVarName<"<num>">;
 def ansi : Flag<["-", "--"], "ansi">;
----------------
These are user facing options, not cc1 flags? I wouldn't expect these to be exposed to users, and they would just be implied by -g. Is there a need for this? Even if there is some need for these, I don't see anything testing for these with only -g

================
Comment at: lib/CodeGen/CGCall.cpp:1601
@@ +1600,3 @@
+    if (CodeGenOpts.AMDGPUToolsInsertNopsOpt)
+      FuncAttrs.addAttribute("amdgpu_tools_insert_nops");
+    if (CodeGenOpts.AMDGPUToolsNumReservedVGPROpt)
----------------
I've been trying to switch to consistently using the '-' separates words conventions most function attributes use.

================
Comment at: test/CodeGenOpenCL/amdgpu-tools-attrs.cl:1
@@ +1,2 @@
+// RUN: %clang_cc1 -triple amdgcn-- -target-cpu kaveri -O0 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -O0 -emit-llvm -verify -o - %s | FileCheck -check-prefix=X86 %s
----------------
I all of the run lines should use amdhsa triple


http://reviews.llvm.org/D17764