Strip function parameter for entry and patch constant function. (micr…

…osoft#20) Strip function parameter for entry and patch constant function * Update error message for ValidationTest::MultiDimArray * Merge remote-tracking branch 'origin/master' # Conflicts: # lib/HLSL/DxilValidation.cpp # tools/clang/unittests/HLSL/ValidationTest.cpp * Update error message for ValidationTest::MultiDimArray Update error message for ValidationTest::MultiDimArray * Fix merge error. (microsoft#3) * Delete local file (microsoft#4) * Fix merge error. * Delete local file * Strip function parameter for entry and patch constant function. (microsoft#5)
Keenuts · Jan 20, 2017 · 5623234 · 5623234
1 parent 43aebaf
commit 5623234
Show file tree

Hide file tree

Showing 9 changed files with 154 additions and 101 deletions.
diff --git a/docs/DXIL.rst b/docs/DXIL.rst
@@ -2124,7 +2124,7 @@ DECL.NOTUSEDEXTERNAL                  External declaration should not be used
 DECL.USEDEXTERNALFUNCTION             External function must be used
 DECL.USEDINTERNAL                     Internal declaration must be used
 FLOW.DEADLOOP                         Loop must have break
-FLOW.FUNCTIONCALL                     Function call on user defined function with parameter is not permitted
+FLOW.FUNCTIONCALL                     Function with parameter is not permitted
 FLOW.NORECUSION                       Recursion is not permitted
 FLOW.REDUCIBLE                        Execution flow must be reducible
 INSTR.ALLOWED                         Instructions must be of an allowed type

diff --git a/include/dxc/HLSL/DxilValidation.h b/include/dxc/HLSL/DxilValidation.h
@@ -147,7 +147,7 @@ enum class ValidationRule : unsigned {
 
   // Program flow
   FlowDeadLoop, // Loop must have break
-  FlowFunctionCall, // Function call on user defined function with parameter is not permitted
+  FlowFunctionCall, // Function with parameter is not permitted
   FlowNoRecusion, // Recursion is not permitted
   FlowReducible, // Execution flow must be reducible
 

diff --git a/lib/HLSL/DxilGenerationPass.cpp b/lib/HLSL/DxilGenerationPass.cpp
@@ -2496,6 +2496,39 @@ INITIALIZE_PASS(HLEnsureMetadata, "hlsl-hlensure", "HLSL High-Level Metadata Ens
 ///////////////////////////////////////////////////////////////////////////////
 
 namespace {
+
+Function *StripFunctionParameter(Function *F, DxilModule &DM,
+    DenseMap<const Function *, DISubprogram *> &FunctionDIs) {
+  Module &M = *DM.GetModule();
+  Type *VoidTy = Type::getVoidTy(M.getContext());
+  FunctionType *FT = FunctionType::get(VoidTy, false);
+  for (auto &arg : F->args()) {
+    if (!arg.user_empty())
+      return nullptr;
+  }
+
+  Function *NewFunc = Function::Create(FT, F->getLinkage(), F->getName());
+  M.getFunctionList().insert(F, NewFunc);
+  // Splice the body of the old function right into the new function.
+  NewFunc->getBasicBlockList().splice(NewFunc->begin(), F->getBasicBlockList());
+
+  // Patch the pointer to LLVM function in debug info descriptor.
+  auto DI = FunctionDIs.find(F);
+  if (DI != FunctionDIs.end()) {
+    DISubprogram *SP = DI->second;
+    SP->replaceFunction(NewFunc);
+    // Ensure the map is updated so it can be reused on subsequent argument
+    // promotions of the same function.
+    FunctionDIs.erase(DI);
+    FunctionDIs[NewFunc] = SP;
+  }
+  NewFunc->takeName(F);
+  DM.GetTypeSystem().EraseFunctionAnnotation(F);
+  F->eraseFromParent();
+  DM.GetTypeSystem().AddFunctionAnnotation(NewFunc);
+  return NewFunc;
+}
+
 class DxilEmitMetadata : public ModulePass {
 public:
   static char ID; // Pass identification, replacement for typeid
@@ -2586,8 +2619,26 @@ class DxilEmitMetadata : public ModulePass {
         }
       }
 
-      M.GetDxilModule().CollectShaderFlags(); // Update flags to reflect any changes.
-      M.GetDxilModule().EmitDxilMetadata();
+      DxilModule &DM = M.GetDxilModule();
+      DenseMap<const Function *, DISubprogram *> FunctionDIs =
+          makeSubprogramMap(M);
+      if (Function *PatchConstantFunc = DM.GetPatchConstantFunction()) {
+        PatchConstantFunc =
+            StripFunctionParameter(PatchConstantFunc, DM, FunctionDIs);
+        if (PatchConstantFunc)
+          DM.SetPatchConstantFunction(PatchConstantFunc);
+      }
+
+      if (Function *EntryFunc = DM.GetEntryFunction()) {
+        StringRef Name = DM.GetEntryFunctionName();
+        EntryFunc->setName(Name);
+        EntryFunc = StripFunctionParameter(EntryFunc, DM, FunctionDIs);
+        if (EntryFunc)
+          DM.SetEntryFunction(EntryFunc);
+      }
+
+      DM.CollectShaderFlags(); // Update flags to reflect any changes.
+      DM.EmitDxilMetadata();
       return true;
     }
 

diff --git a/lib/HLSL/DxilValidation.cpp b/lib/HLSL/DxilValidation.cpp
@@ -216,7 +216,7 @@ const char *hlsl::GetValidationRuleText(ValidationRule value) {
     case hlsl::ValidationRule::FlowReducible: return "Execution flow must be reducible";
     case hlsl::ValidationRule::FlowNoRecusion: return "Recursion is not permitted";
     case hlsl::ValidationRule::FlowDeadLoop: return "Loop must have break";
-    case hlsl::ValidationRule::FlowFunctionCall: return "Function call on user defined function with parameter %0 is not permitted, it should be inlined";
+    case hlsl::ValidationRule::FlowFunctionCall: return "Function %0 with parameter is not permitted, it should be inlined";
     case hlsl::ValidationRule::DeclDxilNsReserved: return "Declaration '%0' uses a reserved prefix";
     case hlsl::ValidationRule::DeclDxilFnExtern: return "External function '%0' is not a DXIL function";
     case hlsl::ValidationRule::DeclUsedInternal: return "Internal declaration '%0' is unused";
@@ -2529,12 +2529,9 @@ static void ValidateFunction(Function &F, ValidationContext &ValCtx) {
   if (F.isDeclaration()) {
     ValidateExternalFunction(&F, ValCtx);
   } else {
-    if (&F != ValCtx.DxilMod.GetEntryFunction() &&
-        &F != ValCtx.DxilMod.GetPatchConstantFunction()) {
-      if (!F.arg_empty())
-        ValCtx.EmitFormatError(ValidationRule::FlowFunctionCall,
-                               {F.getName().str().c_str()});
-    }
+    if (!F.arg_empty())
+      ValCtx.EmitFormatError(ValidationRule::FlowFunctionCall,
+                             {F.getName().str().c_str()});
 
     DxilFunctionAnnotation *funcAnnotation =
         ValCtx.DxilMod.GetTypeSystem().GetFunctionAnnotation(&F);

diff --git a/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp b/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp
@@ -4133,35 +4133,23 @@ void SROA_Parameter_HLSL::flattenArgument(
 void SROA_Parameter_HLSL::moveFunctionBody(Function *F, Function *flatF) {
   bool updateRetType = F->getReturnType() != flatF->getReturnType();
 
-  // Clone blocks and move instructions.
-  std::unordered_map<Value *, Value *> VMap;
-  Function::BasicBlockListType &blockList = F->getBasicBlockList();
-  for (BasicBlock &BB : blockList) {
-    BasicBlock *NewBB = BasicBlock::Create(BB.getContext(), "", flatF);
-    if (BB.hasName()) NewBB->setName(BB.getName());
-    // Add basic block mapping.
-    VMap[&BB] = NewBB;
-    // Update block uses.
-    BB.replaceAllUsesWith(NewBB);
-    // Move intrinsictions to NewBB.
-    NewBB->getInstList().splice(NewBB->begin(), BB.getInstList());
-  }
+  // Splice the body of the old function right into the new function.
+  flatF->getBasicBlockList().splice(flatF->begin(), F->getBasicBlockList());
+
   // Update Block uses.
-  for (BasicBlock &BB : blockList) {
-    BasicBlock *NewBB = cast<BasicBlock>(VMap[&BB]);
-    if (updateRetType) {
-      // Replace ret with ret void.
-      if (ReturnInst *RI = dyn_cast<ReturnInst>(NewBB->getTerminator())) {
-        // Create store for return.
-        IRBuilder<> Builder(RI);
-        Builder.CreateRetVoid();
-        RI->eraseFromParent();
+  if (updateRetType) {
+    for (BasicBlock &BB : flatF->getBasicBlockList()) {
+      if (updateRetType) {
+        // Replace ret with ret void.
+        if (ReturnInst *RI = dyn_cast<ReturnInst>(BB.getTerminator())) {
+          // Create store for return.
+          IRBuilder<> Builder(RI);
+          Builder.CreateRetVoid();
+          RI->eraseFromParent();
+        }
       }
     }
   }
-
-  // Cleanup the original function.
-  F->getBasicBlockList().clear();  
 }
 
 static void SplitArrayCopy(Value *V) {

diff --git a/tools/clang/test/CodeGenHLSL/recursive.hlsl b/tools/clang/test/CodeGenHLSL/recursive.hlsl
@@ -1,7 +1,7 @@
 // RUN: %dxc -E main -T ps_6_0 %s | FileCheck %s
 
 // CHECK: Recursion is not permitted
-// CHECK: Function call on user defined function
+// CHECK: with parameter is not permitted
 
 void test_inout(inout float4 m, float4 a) 
 {

diff --git a/tools/clang/unittests/HLSL/ExtensionTest.cpp b/tools/clang/unittests/HLSL/ExtensionTest.cpp
@@ -455,7 +455,7 @@ TEST_F(ExtensionTest, IntrinsicWhenAvailableThenUsed) {
   // - second argument is float, ie it got scalarized
   VERIFY_IS_TRUE(
     disassembly.npos !=
-    disassembly.find("call void @\"test.\\01?test_proc@hlsl@@YAXV?$vector@M$01@@@Z.r\"(i32 2, float %6)"));
+    disassembly.find("call void @\"test.\\01?test_proc@hlsl@@YAXV?$vector@M$01@@@Z.r\"(i32 2, float"));
   VERIFY_IS_TRUE(
     disassembly.npos !=
     disassembly.find("call float @\"test.\\01?test_fn@hlsl@@YA?AV?$vector@M$01@@V2@@Z.r\"(i32 1, float"));