[PATCH] D147657: [Sema] Fix reporting of invalid shader attribute on HLSL entry function

[Richard Sandiford via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG03a9a1e664de: [Sema] Fix reporting of invalid shader 
attribute on HLSL entry function (authored by rsandifo-arm).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147657/new/

https://reviews.llvm.org/D147657

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaHLSL/entry_shader.hlsl


Index: clang/test/SemaHLSL/entry_shader.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/entry_shader.hlsl
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry 
foo  -o - %s -DSHADER='"anyHit"' -verify
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry 
foo  -o - %s -DSHADER='"compute"'
+
+// expected-error@+1 {{'shader' attribute on entry function does not match the 
pipeline stage}}
+[numthreads(1,1,1), shader(SHADER)]
+void foo() {
+
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10205,14 +10205,19 @@
   CheckHLSLEntryPoint(NewFD);
   if (!NewFD->isInvalidDecl()) {
 auto Env = TargetInfo.getTriple().getEnvironment();
-AttributeCommonInfo AL(NewFD->getBeginLoc());
 HLSLShaderAttr::ShaderType ShaderType =
 static_cast(
 hlsl::getStageFromEnvironment(Env));
 // To share code with HLSLShaderAttr, add HLSLShaderAttr to entry
 // function.
-if (HLSLShaderAttr *Attr = mergeHLSLShaderAttr(NewFD, AL, ShaderType))
-  NewFD->addAttr(Attr);
+if (HLSLShaderAttr *NT = NewFD->getAttr()) {
+  if (NT->getType() != ShaderType)
+Diag(NT->getLocation(), diag::err_hlsl_entry_shader_attr_mismatch)
+<< NT;
+} else {
+  NewFD->addAttr(HLSLShaderAttr::Create(Context, ShaderType,
+NewFD->getBeginLoc()));
+}
   }
 }
 // HLSL does not support specifying an address space on a function return
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11752,6 +11752,8 @@
"attribute %0 only applies to a field or parameter of type '%1'">;
 def err_hlsl_attr_invalid_ast_node : Error<
"attribute %0 only applies to %1">;
+def err_hlsl_entry_shader_attr_mismatch : Error<
+   "%0 attribute on entry function does not match the pipeline stage">;
 def err_hlsl_numthreads_argument_oor : Error<"argument '%select{X|Y|Z}0' to 
numthreads attribute cannot exceed %1">;
 def err_hlsl_numthreads_invalid : Error<"total number of threads cannot exceed 
%0">;
 def err_hlsl_missing_numthreads : Error<"missing numthreads attribute for %0 
shader entry">;


Index: clang/test/SemaHLSL/entry_shader.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/entry_shader.hlsl
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry foo  -o - %s -DSHADER='"anyHit"' -verify
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry foo  -o - %s -DSHADER='"compute"'
+
+// expected-error@+1 {{'shader' attribute on entry function does not match the pipeline stage}}
+[numthreads(1,1,1), shader(SHADER)]
+void foo() {
+
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10205,14 +10205,19 @@
   CheckHLSLEntryPoint(NewFD);
   if (!NewFD->isInvalidDecl()) {
 auto Env = TargetInfo.getTriple().getEnvironment();
-AttributeCommonInfo AL(NewFD->getBeginLoc());
 HLSLShaderAttr::ShaderType ShaderType =
 static_cast(
 hlsl::getStageFromEnvironment(Env));
 // To share code with HLSLShaderAttr, add HLSLShaderAttr to entry
 // function.
-if (HLSLShaderAttr *Attr = mergeHLSLShaderAttr(NewFD, AL, ShaderType))
-  NewFD->addAttr(Attr);
+if (HLSLShaderAttr *NT = NewFD->getAttr()) {
+  if (NT->getType() != ShaderType)
+Diag(NT->getLocation(), diag::err_hlsl_entry_shader_attr_mismatch)
+<< NT;
+} else {
+  NewFD->addAttr(HLSLShaderAttr::Create(Context, ShaderType,
+NewFD->getBeginLoc()));
+}
   }
 }
 // HLSL does not support specifying an address space on a function return
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===

[PATCH] D147657: [Sema] Fix reporting of invalid shader attribute on HLSL entry function

[Xiang Li via Phabricator via cfe-commits]

python3kgae accepted this revision.
python3kgae added a comment.
This revision is now accepted and ready to land.

Thanks for fix the issue.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147657/new/

https://reviews.llvm.org/D147657

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147657: [Sema] Fix reporting of invalid shader attribute on HLSL entry function

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm created this revision.
rsandifo-arm added reviewers: python3kgae, erichkeane.
Herald added a subscriber: Anastasia.
Herald added a project: All.
rsandifo-arm requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

If the HLSL entry function had a shader attribute that conflicted
with the pipeline stage specified in the target triple, Clang
would emit:

  error: (null) attribute parameters do not match the previous declaration
  conflicting attribute is here

(where the second line doesn't reference an attribute).

This was because the code constructed a dummy attribute that had
only a source location, but no kind or syntax.

Noticed while doing some changes to the attribute handling.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147657

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaHLSL/entry_shader.hlsl


Index: clang/test/SemaHLSL/entry_shader.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/entry_shader.hlsl
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry 
foo  -o - %s -DSHADER='"anyHit"' -verify
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry 
foo  -o - %s -DSHADER='"compute"'
+
+// expected-error@+1 {{'shader' attribute on entry function does not match the 
pipeline stage}}
+[numthreads(1,1,1), shader(SHADER)]
+void foo() {
+
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10205,14 +10205,19 @@
   CheckHLSLEntryPoint(NewFD);
   if (!NewFD->isInvalidDecl()) {
 auto Env = TargetInfo.getTriple().getEnvironment();
-AttributeCommonInfo AL(NewFD->getBeginLoc());
 HLSLShaderAttr::ShaderType ShaderType =
 static_cast(
 hlsl::getStageFromEnvironment(Env));
 // To share code with HLSLShaderAttr, add HLSLShaderAttr to entry
 // function.
-if (HLSLShaderAttr *Attr = mergeHLSLShaderAttr(NewFD, AL, ShaderType))
-  NewFD->addAttr(Attr);
+if (HLSLShaderAttr *NT = NewFD->getAttr()) {
+  if (NT->getType() != ShaderType)
+Diag(NT->getLocation(), diag::err_hlsl_entry_shader_attr_mismatch)
+<< NT;
+} else {
+  NewFD->addAttr(HLSLShaderAttr::Create(Context, ShaderType,
+NewFD->getBeginLoc()));
+}
   }
 }
 // HLSL does not support specifying an address space on a function return
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11752,6 +11752,8 @@
"attribute %0 only applies to a field or parameter of type '%1'">;
 def err_hlsl_attr_invalid_ast_node : Error<
"attribute %0 only applies to %1">;
+def err_hlsl_entry_shader_attr_mismatch : Error<
+   "%0 attribute on entry function does not match the pipeline stage">;
 def err_hlsl_numthreads_argument_oor : Error<"argument '%select{X|Y|Z}0' to 
numthreads attribute cannot exceed %1">;
 def err_hlsl_numthreads_invalid : Error<"total number of threads cannot exceed 
%0">;
 def err_hlsl_missing_numthreads : Error<"missing numthreads attribute for %0 
shader entry">;


Index: clang/test/SemaHLSL/entry_shader.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/entry_shader.hlsl
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry foo  -o - %s -DSHADER='"anyHit"' -verify
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry foo  -o - %s -DSHADER='"compute"'
+
+// expected-error@+1 {{'shader' attribute on entry function does not match the pipeline stage}}
+[numthreads(1,1,1), shader(SHADER)]
+void foo() {
+
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10205,14 +10205,19 @@
   CheckHLSLEntryPoint(NewFD);
   if (!NewFD->isInvalidDecl()) {
 auto Env = TargetInfo.getTriple().getEnvironment();
-AttributeCommonInfo AL(NewFD->getBeginLoc());
 HLSLShaderAttr::ShaderType ShaderType =
 static_cast(
 hlsl::getStageFromEnvironment(Env));
 // To share code with HLSLShaderAttr, add HLSLShaderAttr to entry
 // function.
-if (HLSLShaderAttr *Attr = mergeHLSLShaderAttr(NewFD, AL, ShaderType))
-  NewFD->addAttr(Attr);
+if (HLSLShaderAttr *NT = NewFD->getAttr()) {
+  if (NT->getType() != ShaderType)
+Diag(NT->getLocation(), diag::