put hlsl specific warnings in diagnostic groups (#3026)

tools/clang/include/clang/Basic/DiagnosticGroups.td

@@ -781,4 +781,9 @@ def FutureCompat : DiagGroup<"future-compat">;
 // HLSL Change Starts - HLSL diagnostic groups
 def HLSLEffectsSyntax : DiagGroup<"effects-syntax">;
 def HLSLForRedefinition : DiagGroup<"for-redefinition">;
+def HLSLAttributeStatement : DiagGroup<"attribute-statement">;
+def HLSLAttributeType : DiagGroup<"attribute-type">;
+def HLSLSpecifierOverride : DiagGroup<"specifier-override">;
+def HLSLPackOffsetOverride : DiagGroup<"packoffset-override">;
+def HLSLCommaInInit : DiagGroup<"comma-in-init">;
 // HLSL Change Ends

tools/clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -7604,11 +7604,13 @@ def err_hlsl_conditional_result_comptype_mismatch : Error<
   "conditional operator result component types mismatch.">;
 def note_field_type_usage : Note<"usage of %0 found in field %1 of type %2">;
 def warn_hlsl_attribute_expects_uint_literal : Warning<
-  "attribute %0 must have a uint literal argument">;
+  "attribute %0 must have a uint literal argument">,
+  InGroup<HLSLAttributeType>;
 def err_hlsl_attribute_expects_float_literal : Error<
   "attribute %0 must have a float literal argument">;
 def warn_hlsl_comma_in_init : Warning<
-  "comma expression used where a constructor list may have been intended">;
+  "comma expression used where a constructor list may have been intended">,
+  InGroup<HLSLCommaInInit>;
 def warn_hlsl_incorrect_bind_semantic: Warning< // Is an error in fxc in some cases, but is sometimes ignored
   "invalid register specification, expected %0 binding">;
 def err_hlsl_incorrect_bind_semantic: Error< // Is an error in fxc in some cases, but is sometimes ignored
@@ -7639,17 +7641,23 @@ def err_hlsl_usage_not_on_parameter : Error<
 def warn_hlsl_for_redefinition : Warning<"redefinition of %0 shadows declaration in the outer scope; most recent declaration will be used">,
   InGroup<HLSLForRedefinition>;
 def warn_hlsl_for_redefinition_different_type : Warning<
-  "redefinition of %0 with a different type%diff{: $ vs $|}1,2 shadows declaration in the outer scope; most recent declaration will be used">;
+  "redefinition of %0 with a different type%diff{: $ vs $|}1,2 shadows declaration in the outer scope; most recent declaration will be used">,
+  InGroup<HLSLForRedefinition>;
 def warn_hlsl_specifier_overridden : Warning<
-  "%0 will be overridden by %1">;
+  "%0 will be overridden by %1">,
+  InGroup<HLSLSpecifierOverride>;
 def warn_hlsl_packoffset_overridden : Warning<
-  "packoffset is overridden by another packoffset annotation">;
+  "packoffset is overridden by another packoffset annotation">,
+  InGroup<HLSLPackOffsetOverride>;
 def warn_hlsl_unsupported_statement_for_if_switch_attribute : Warning<
-  "attribute %0 can only be applied to 'if' and 'switch' statements">;
+  "attribute %0 can only be applied to 'if' and 'switch' statements">,
+  InGroup<HLSLAttributeStatement>;
 def warn_hlsl_unsupported_statement_for_switch_attribute : Warning<
-  "attribute %0 can only be applied to 'switch' statements">;
+  "attribute %0 can only be applied to 'switch' statements">,
+  InGroup<HLSLAttributeStatement>;
 def warn_hlsl_unsupported_statement_for_loop_attribute : Warning<
-  "attribute %0 can only be applied to 'for', 'while' and 'do' loop statements">;
+  "attribute %0 can only be applied to 'for', 'while' and 'do' loop statements">,
+  InGroup<HLSLAttributeStatement>;
 def err_hlsl_matrix_layout_wrong_type : Error<
   "%0 can only be used with a matrix type">;
 def warn_hlsl_effect_object : Warning <

tools/clang/lib/SPIRV/SpirvEmitter.cpp

@@ -1736,11 +1736,7 @@ void SpirvEmitter::doIfStmt(const IfStmt *ifStmt,
       selectionControl = spv::SelectionControlMask::Flatten;
       break;
     default:
-      if (!spirvOptions.noWarnIgnoredFeatures) {
-        emitWarning("unknown if statement attribute '%0' ignored",
-                    attribute->getLocation())
-            << attribute->getSpelling();
-      }
+      // warning emitted in hlsl::ProcessStmtAttributeForHLSL
       break;
     }
   }

tools/clang/test/HLSL/attributes.hlsl

@@ -19,6 +19,198 @@ int loop_before_return() {
   return 0;
 }
 
+int loop_before_if(int a) {
+  [loop] // expected-warning {{attribute 'loop' can only be applied to 'for', 'while' and 'do' loop statements}}
+  if (a > 0) return -a;
+  return a;
+}
+
+int loop_before_switch(int a) {
+  [loop] // expected-warning {{attribute 'loop' can only be applied to 'for', 'while' and 'do' loop statements}}
+  switch (a) {
+    case 0:
+      return 1;
+      break;
+  }
+  return 0;
+}
+
+int fastopt_before_if(int a) {
+  [fastopt] // expected-warning {{attribute 'fastopt' can only be applied to 'for', 'while' and 'do' loop statements}}
+  if (a > 0) return -a;
+  return a;
+}
+
+int fastopt_before_switch(int a) {
+  [fastopt] // expected-warning {{attribute 'fastopt' can only be applied to 'for', 'while' and 'do' loop statements}}
+  switch (a) {
+    case 0:
+      return 1;
+      break;
+  }
+  return 0;
+}
+
+int unroll_before_if(int a) {
+  [unroll] // expected-warning {{attribute 'unroll' can only be applied to 'for', 'while' and 'do' loop statements}}
+  if (a > 0) return -a;
+  return a;
+}
+
+int unroll_before_switch(int a) {
+  [unroll] // expected-warning {{attribute 'unroll' can only be applied to 'for', 'while' and 'do' loop statements}}
+  switch (a) {
+    case 0:
+      return 1;
+      break;
+  }
+  return 0;
+}
+
+int allow_uav_condition_before_if(int a) {
+  [allow_uav_condition] // expected-warning {{attribute 'allow_uav_condition' can only be applied to 'for', 'while' and 'do' loop statements}}
+  if (a > 0) return -a;
+  return a;
+}
+
+int allow_uav_condition_before_switch(int a) {
+  [allow_uav_condition] // expected-warning {{attribute 'allow_uav_condition' can only be applied to 'for', 'while' and 'do' loop statements}}
+  switch (a) {
+    case 0:
+      return 1;
+      break;
+  }
+  return 0;
+}
+
+int branch_before_for() {
+  int result = 0;
+  [branch] // expected-warning {{attribute 'branch' can only be applied to 'if' and 'switch' statements}}
+  for (int i = 0; i < 10; i++) result++;
+  return result;
+}
+
+int branch_before_while() {
+  int result = 0;
+  int i = 0;
+  [branch] // expected-warning {{attribute 'branch' can only be applied to 'if' and 'switch' statements}}
+  while(i < 10) {
+    result++;
+    i++;
+  }
+  return result;
+}
+
+int branch_before_do() {
+  int result = 0;
+  int i = 0;
+  [branch] // expected-warning {{attribute 'branch' can only be applied to 'if' and 'switch' statements}}
+  do {
+    result++;
+    i++;
+  } while(i < 10);
+  return result;
+}
+
+int flatten_before_for() {
+  int result = 0;
+  [flatten] // expected-warning {{attribute 'flatten' can only be applied to 'if' and 'switch' statements}}
+  for (int i = 0; i < 10; i++) result++;
+  return result;
+}
+
+int flatten_before_while() {
+  int result = 0;
+  int i = 0;
+  [flatten] // expected-warning {{attribute 'flatten' can only be applied to 'if' and 'switch' statements}}
+  while(i < 10) {
+    result++;
+    i++;
+  }
+  return result;
+}
+
+int flatten_before_do() {
+  int result = 0;
+  int i = 0;
+  [flatten] // expected-warning {{attribute 'flatten' can only be applied to 'if' and 'switch' statements}}
+  do {
+    result++;
+    i++;
+  } while(i < 10);
+  return result;
+}
+
+int forcecase_before_for() {
+  int result = 0;
+  [forcecase] // expected-warning {{attribute 'forcecase' can only be applied to 'switch' statements}}
+  for (int i = 0; i < 10; i++) result++;
+  return result;
+}
+
+int forcecase_before_while() {
+  int result = 0;
+  int i = 0;
+  [forcecase] // expected-warning {{attribute 'forcecase' can only be applied to 'switch' statements}}
+  while(i < 10) {
+    result++;
+    i++;
+  }
+  return result;
+}
+
+int forcecase_before_do() {
+  int result = 0;
+  int i = 0;
+  [forcecase] // expected-warning {{attribute 'forcecase' can only be applied to 'switch' statements}}
+  do {
+    result++;
+    i++;
+  } while(i < 10);
+  return result;
+}
+
+int forcecase_before_if(int a) {
+  [forcecase] // expected-warning {{attribute 'forcecase' can only be applied to 'switch' statements}}
+  if (a > 0) return -a;
+  return a;
+}
+
+int call_before_for() {
+  int result = 0;
+  [call] // expected-warning {{attribute 'call' can only be applied to 'switch' statements}}
+  for (int i = 0; i < 10; i++) result++;
+  return result;
+}
+
+int call_before_while() {
+  int result = 0;
+  int i = 0;
+  [call] // expected-warning {{attribute 'call' can only be applied to 'switch' statements}}
+  while(i < 10) {
+    result++;
+    i++;
+  }
+  return result;
+}
+
+int call_before_do() {
+  int result = 0;
+  int i = 0;
+  [call] // expected-warning {{attribute 'call' can only be applied to 'switch' statements}}
+  do {
+    result++;
+    i++;
+  } while(i < 10);
+  return result;
+}
+
+int call_before_if(int a) {
+  [call] // expected-warning {{attribute 'call' can only be applied to 'switch' statements}}
+  if (a > 0) return -a;
+  return a;
+}
+
 int short_unroll() {
   int result = 2;
 
@@ -742,4 +934,4 @@ bool Test_Unknown() {
   [unknown] bool b1 = false;                  /* expected-warning {{unknown attribute 'unknown' ignored}} fxc-pass {{}} */
   [unknown(1)] bool b2 = false;               /* expected-warning {{unknown attribute 'unknown' ignored}} fxc-pass {{}} */
   [unknown(1)] while (g_bool) return g_bool;  /* expected-warning {{unknown attribute 'unknown' ignored}} fxc-pass {{}} */
-}
+}

tools/clang/test/HLSLFileCheck/hlsl/diagnostics/warnings/Wno-attribute-statement.hlsl

@@ -0,0 +1,17 @@
+// RUN: %dxc -T vs_6_0 -Wno-attribute-statement %s | FileCheck %s
+
+// Make sure the specified warning gets turned off
+
+// This function has no output semantic on purpose in order to produce an error,
+// otherwise, the warnings will not be captured in the output for FileCheck.
+float main() {
+
+// attribute %0 can only be applied to 'if' and 'switch' statements
+// CHECK-NOT: statement
+  [branch]
+  do {} while(true);
+
+  return 0;
+}
+
+// CHECK: error: Semantic must be defined

tools/clang/test/HLSLFileCheck/hlsl/diagnostics/warnings/Wno-attribute-type.hlsl

@@ -0,0 +1,14 @@
+// RUN: %dxc -T vs_6_0 -Wno-attribute-type %s | FileCheck %s
+// Make sure the specified warning gets turned off
+
+// Compile with vs profile instead of cs on purpose in order to produce an error,
+// otherwise, the warnings will not be captured in the output for FileCheck.
+
+// attribute %0 must have a uint literal argument
+// CHECK-NOT: uint literal argument
+[numthreads(1.0f, 0, 0)]
+void main() {
+  return;
+}
+
+// CHECK: error: attribute numthreads only

tools/clang/test/HLSLFileCheck/hlsl/diagnostics/warnings/Wno-comma-in-init.hlsl

@@ -0,0 +1,17 @@
+// RUN: %dxc -T vs_6_0 -Wno-comma-in-init %s | FileCheck %s
+
+// Make sure the specified warning gets turned off
+
+// This function has no output semantic on purpose in order to produce an error,
+// otherwise, the warnings will not be captured in the output for FileCheck.
+float main() {
+
+// comma expression used where a constructor list may have been intended
+// CHECK-NOT: comma
+  int a = 1, b = 2;
+  int c = (a, b);
+
+  return 0;
+}
+
+// CHECK: error: Semantic must be defined

tools/clang/test/HLSLFileCheck/hlsl/diagnostics/warnings/Wno-packoffset-override.hlsl

@@ -0,0 +1,18 @@
+// RUN: %dxc -T vs_6_0 -Wno-packoffset-override %s | FileCheck %s
+
+// Make sure the specified warning gets turned off
+
+cbuffer CBuf
+{
+// packoffset is overridden by another packoffset annotation
+// CHECK-NOT: overridden
+  float4 Element100 : packoffset(c100) : packoffset(c2);
+}
+
+// This function has no output semantic on purpose in order to produce an error,
+// otherwise, the warnings will not be captured in the output for FileCheck.
+float main() {
+  return 0;
+}
+
+// CHECK: error: Semantic must be defined

tools/clang/test/HLSLFileCheck/hlsl/diagnostics/warnings/Wno-specifier-override.hlsl

@@ -0,0 +1,18 @@
+// RUN: %dxc -T vs_6_0 -Wno-specifier-override %s | FileCheck %s
+
+// Make sure the specified warning gets turned off
+
+struct Struct
+{
+// %0 will be overridden by %1
+// CHECK-NOT: overridden
+  sample centroid float attr : ATTR;
+};
+
+// This function has no output semantic on purpose in order to produce an error,
+// otherwise, the warnings will not be captured in the output for FileCheck.
+float main() {
+  return 0;
+}
+
+// CHECK: error: Semantic must be defined