moves the patch constant fn validation to the backend. (#754)

* moves the patch constant fn validation to the backend.

* addresses comments

* fixes broken test

tools/clang/lib/CodeGen/CGHLSLMS.cpp

@@ -4083,12 +4083,10 @@ void CGMSHLSLRuntime::SetPatchConstantFunctionWithAttr(
   }
 
   if (Entry->second.NumOverloads != 1) {
-    DXASSERT(false,
-        "hlsl::DiagnoseTranslationUnit used to check for this condition.");
     DiagnosticsEngine &Diags = CGM.getDiags();
     unsigned DiagID =
       Diags.getCustomDiagID(DiagnosticsEngine::Warning,
-        "Multiple functions match patchconstantfunc %0.");
+        "Multiple overloads of patchconstantfunc %0.");
     unsigned NoteID =
       Diags.getCustomDiagID(DiagnosticsEngine::Note,
         "This overload was selected.");

tools/clang/lib/Sema/SemaHLSL.cpp

@@ -8930,13 +8930,6 @@ void hlsl::DiagnoseTranslationUnit(clang::Sema *self) {
     if (const HLSLPatchConstantFuncAttr *Attr =
             pEntryPointDecl->getAttr<HLSLPatchConstantFuncAttr>()) {
       NameLookup NL = GetSingleFunctionDeclByName(self, Attr->getFunctionName(), /*checkPatch*/ true);
-      if (NL.Found && NL.Other) {
-        unsigned id = Diags.getCustomDiagID(clang::DiagnosticsEngine::Level::Error,
-          "ambiguous patch constant function");
-        Diags.Report(NL.Found->getSourceRange().getBegin(), id);
-        Diags.Report(NL.Other->getLocation(), diag::note_previous_definition);
-        return;
-      }
       if (!NL.Found || !NL.Found->hasBody()) {
         unsigned id = Diags.getCustomDiagID(clang::DiagnosticsEngine::Level::Error,
           "missing patch function definition");

tools/clang/test/CodeGenHLSL/SimpleHs10.hlsl

@@ -0,0 +1,97 @@
+// RUN: %dxc -E main -T hs_6_1  %s 2>&1 | StdErrCheck %s
+
+// Same as SimpleHS11.hlsl, except that we only verify StdErr for the warning
+// message.
+
+// CHECK: Multiple overloads of patchconstantfunc HSPerPatchFunc.
+// CHECK: 87:16: note: This overload was selected
+
+//--------------------------------------------------------------------------------------
+// SimpleTessellation.hlsl
+//
+// Advanced Technology Group (ATG)
+// Copyright (C) Microsoft Corporation. All rights reserved.
+//--------------------------------------------------------------------------------------
+
+
+struct PSSceneIn
+{
+    float4 pos     : SV_Position;
+    float2 tex     : TEXCOORD0;
+    float3 norm    : NORMAL;
+    uint   RTIndex : SV_RenderTargetArrayIndex;
+};
+
+
+//////////////////////////////////////////////////////////////////////////////////////////
+// Simple forwarding Tessellation shaders
+
+struct HSPerVertexData
+{
+    // This is just the original vertex verbatim. In many real life cases this would be a
+    // control point instead
+    PSSceneIn v;
+};
+
+struct HSPerPatchData
+{
+    // We at least have to specify tess factors per patch
+    // As we're tesselating triangles, there will be 4 tess factors
+    // In real life case this might contain face normal, for example
+	float	edges[3] : SV_TessFactor;
+	float	inside   : SV_InsideTessFactor;
+};
+
+// This function has the same name as the patch constant function, but
+// its signature prevents it from being a candidate.
+float4 HSPerPatchFunc()
+{
+    return 1.8;
+}
+
+// This overload is a patch constant function candidate because it has an
+// output with the SV_TessFactor semantic. However, the compiler should
+// *not* select it because there is another overload defined later in this
+// translation unit (which is the old compiler's behavior). If it did, then
+// the semantic checker will report an error due to this overload's input
+// having 32 elements (versus the expected 3).
+HSPerPatchData HSPerPatchFunc(const InputPatch< PSSceneIn, 32 > points)
+{
+  HSPerPatchData d;
+
+  d.edges[0] = -5;
+  d.edges[1] = -6;
+  d.edges[2] = -7;
+  d.inside = -8;
+
+  return d;
+}
+
+// hull per-control point shader
+[domain("tri")]
+[partitioning("fractional_odd")]
+[outputtopology("triangle_cw")]
+[patchconstantfunc("HSPerPatchFunc")]
+[outputcontrolpoints(3)]
+HSPerVertexData main( const uint id : SV_OutputControlPointID,
+                      const InputPatch< PSSceneIn, 3 > points )
+{
+    HSPerVertexData v;
+
+    // Just forward the vertex
+    v.v = points[ id ];
+
+	return v;
+}
+
+HSPerPatchData HSPerPatchFunc(const InputPatch< PSSceneIn, 3 > points)
+{
+  HSPerPatchData d;
+
+  d.edges[0] = 1;
+  d.edges[1] = 2;
+  d.edges[2] = 3;
+  d.inside = 4;
+
+  return d;
+}

tools/clang/test/CodeGenHLSL/SimpleHs11.hlsl

@@ -0,0 +1,105 @@
+// RUN: %dxc -E main -T hs_6_1  %s 2>&1 | FileCheck %s
+
+// Same as SimpleHS10.hlsl, except that now we check that the compiler didn't
+// lie when it told us which overload it selected.
+
+// CHECK: SV_TessFactor 0
+// CHECK: SV_InsideTessFactor 0
+
+// CHECK: define void @main
+
+// CHECK: define void {{.*}}HSPerPatchFunc
+// CHECK: dx.op.storePatchConstant.f32{{.*}}float 1.0
+// CHECK: dx.op.storePatchConstant.f32{{.*}}float 2.0
+// CHECK: dx.op.storePatchConstant.f32{{.*}}float 3.0
+// CHECK: dx.op.storePatchConstant.f32{{.*}}float 4.0
+
+//--------------------------------------------------------------------------------------
+// SimpleTessellation.hlsl
+//
+// Advanced Technology Group (ATG)
+// Copyright (C) Microsoft Corporation. All rights reserved.
+//--------------------------------------------------------------------------------------
+
+
+struct PSSceneIn
+{
+    float4 pos     : SV_Position;
+    float2 tex     : TEXCOORD0;
+    float3 norm    : NORMAL;
+    uint   RTIndex : SV_RenderTargetArrayIndex;
+};
+
+
+//////////////////////////////////////////////////////////////////////////////////////////
+// Simple forwarding Tessellation shaders
+
+struct HSPerVertexData
+{
+    // This is just the original vertex verbatim. In many real life cases this would be a
+    // control point instead
+    PSSceneIn v;
+};
+
+struct HSPerPatchData
+{
+    // We at least have to specify tess factors per patch
+    // As we're tesselating triangles, there will be 4 tess factors
+    // In real life case this might contain face normal, for example
+	float	edges[3] : SV_TessFactor;
+	float	inside   : SV_InsideTessFactor;
+};
+
+// This function has the same name as the patch constant function, but
+// its signature prevents it from being a candidate.
+float4 HSPerPatchFunc()
+{
+    return 1.8;
+}
+
+// This overload is a patch constant function candidate because it has an
+// output with the SV_TessFactor semantic. However, the compiler should
+// *not* select it because there is another overload defined later in this
+// translation unit (which is the old compiler's behavior). If it did, then
+// the semantic checker will report an error due to this overload's input
+// having 32 elements (versus the expected 3).
+HSPerPatchData HSPerPatchFunc(const InputPatch< PSSceneIn, 32 > points)
+{
+  HSPerPatchData d;
+
+  d.edges[0] = -5;
+  d.edges[1] = -6;
+  d.edges[2] = -7;
+  d.inside = -8;
+
+  return d;
+}
+
+// hull per-control point shader
+[domain("tri")]
+[partitioning("fractional_odd")]
+[outputtopology("triangle_cw")]
+[patchconstantfunc("HSPerPatchFunc")]
+[outputcontrolpoints(3)]
+HSPerVertexData main( const uint id : SV_OutputControlPointID,
+                      const InputPatch< PSSceneIn, 3 > points )
+{
+    HSPerVertexData v;
+
+    // Just forward the vertex
+    v.v = points[ id ];
+
+	return v;
+}
+
+HSPerPatchData HSPerPatchFunc(const InputPatch< PSSceneIn, 3 > points)
+{
+  HSPerPatchData d;
+
+  d.edges[0] = 1;
+  d.edges[1] = 2;
+  d.edges[2] = 3;
+  d.inside = 4;
+
+  return d;
+}

tools/clang/unittests/HLSL/CompilerTest.cpp

@@ -762,6 +762,8 @@ public:
   TEST_METHOD(CodeGenSimpleHS7)
   TEST_METHOD(CodeGenSimpleHS8)
   TEST_METHOD(CodeGenSimpleHS9)
+  TEST_METHOD(CodeGenSimpleHS10)
+  TEST_METHOD(CodeGenSimpleHS11)
   TEST_METHOD(CodeGenSMFail)
   TEST_METHOD(CodeGenSrv_Ms_Load1)
   TEST_METHOD(CodeGenSrv_Ms_Load2)
@@ -4195,6 +4197,14 @@ TEST_F(CompilerTest, CodeGenSimpleHS9) {
   CodeGenTestCheck(L"..\\CodeGenHLSL\\SimpleHS9.hlsl");
 }
 
+TEST_F(CompilerTest, CodeGenSimpleHS10) {
+  CodeGenTestCheck(L"..\\CodeGenHLSL\\SimpleHS10.hlsl");
+}
+
+TEST_F(CompilerTest, CodeGenSimpleHS11) {
+  CodeGenTestCheck(L"..\\CodeGenHLSL\\SimpleHS11.hlsl");
+}
+
 TEST_F(CompilerTest, CodeGenSMFail) {
   CodeGenTestCheck(L"sm-fail.hlsl");
 }