Kaynağa Gözat

Fix ViewID state for flow control and consistent ViewID detection (#484)

* Fix ViewID state for flow control and consistent ViewID detection

1. Different ways of determining ViewID usage led to assert or crash.
Use DxilModule flag to guarantee consistency.  But this required
splitting DxilEmitMetadata pass into two passes, one that finalizes
the module and computes flags run before ComputeViewIdState, and one
that simply writes the metadata.

2. storeOutput in flow control with values not dependent on that flow,
such as literals, would not pick up control flow dependence.
Fix by picking up control flow dependence on storeOutput instructions,
not just the value being written.

* Fix control depenence computation

There was a bug in the implementation of the control dependence algorithm.
We were incorrectly iterating over all descendents in the postdom tree where
the algorithm should only iterate over immediate children.
Tex Riddell 8 yıl önce
ebeveyn
işleme
c9beb84c51

+ 2 - 0
include/dxc/HLSL/DxilGenerationPass.h

@@ -42,6 +42,7 @@ ModulePass *createDxilEliminateOutputDynamicIndexingPass();
 ModulePass *createDxilGenerationPass(bool NotOptimized, hlsl::HLSLExtensionsCodegenHelper *extensionsHelper);
 ModulePass *createHLEmitMetadataPass();
 ModulePass *createHLEnsureMetadataPass();
+ModulePass *createDxilFinalizeModulePass();
 ModulePass *createDxilEmitMetadataPass();
 FunctionPass *createDxilExpandTrigIntrinsicsPass();
 ModulePass *createDxilLoadMetadataPass();
@@ -63,6 +64,7 @@ void initializeDxilEliminateOutputDynamicIndexingPass(llvm::PassRegistry&);
 void initializeDxilGenerationPassPass(llvm::PassRegistry&);
 void initializeHLEnsureMetadataPass(llvm::PassRegistry&);
 void initializeHLEmitMetadataPass(llvm::PassRegistry&);
+void initializeDxilFinalizeModulePass(llvm::PassRegistry&);
 void initializeDxilEmitMetadataPass(llvm::PassRegistry&);
 void initializeDxilExpandTrigIntrinsicsPass(llvm::PassRegistry&);
 void initializeDxilLoadMetadataPass(llvm::PassRegistry&);

+ 20 - 8
lib/HLSL/ComputeViewIdState.cpp

@@ -53,6 +53,7 @@ void DxilViewIdState::Compute() {
   Clear();
 
   const ShaderModel *pSM = m_pModule->GetShaderModel();
+  m_bUsesViewId = m_pModule->m_ShaderFlags.GetViewID();
 
   // 1. Traverse signature MD to determine max packed location.
   DetermineMaxPackedLocation(m_pModule->GetInputSignature(), &m_NumInputSigScalars, 1);
@@ -399,19 +400,30 @@ void DxilViewIdState::CollectValuesContributingToOutputs(EntryInfo &Entry) {
       endRow = SigElem.GetRows() - 1;
     }
 
+    InstructionSetType ContributingInstructionsAllRows;
+    InstructionSetType *pContributingInstructions = &ContributingInstructionsAllRows;
     if (startRow == endRow) {
       // Scalar or indexable with known index.
       unsigned index = GetLinearIndex(SigElem, startRow, col);
-      InstructionSetType &ContributingInstructions = Entry.ContributingInstructions[StreamId][index];
-      CollectValuesContributingToOutputRec(Entry, pContributingValue, ContributingInstructions);
-    } else {
-      // Dynamically indexed output.
-      InstructionSetType ContributingInstructions;
-      CollectValuesContributingToOutputRec(Entry, pContributingValue, ContributingInstructions);
+      pContributingInstructions = &Entry.ContributingInstructions[StreamId][index];
+    }
+
+    CollectValuesContributingToOutputRec(Entry, pContributingValue, *pContributingInstructions);
+
+    // Handle control dependence of this instruction BB.
+    BasicBlock *pBB = CI->getParent();
+    Function *F = pBB->getParent();
+    FuncInfo *pFuncInfo = m_FuncInfo[F].get();
+    const BasicBlockSet &CtrlDepSet = pFuncInfo->CtrlDep.GetCDBlocks(pBB);
+    for (BasicBlock *B : CtrlDepSet) {
+      CollectValuesContributingToOutputRec(Entry, B->getTerminator(), *pContributingInstructions);
+    }
 
+    if (pContributingInstructions == &ContributingInstructionsAllRows) {
+      // Write dynamically indexed output contributions to all rows.
       for (int row = startRow; row <= endRow; row++) {
         unsigned index = GetLinearIndex(SigElem, row, col);
-        Entry.ContributingInstructions[StreamId][index].insert(ContributingInstructions.begin(), ContributingInstructions.end());
+        Entry.ContributingInstructions[StreamId][index].insert(ContributingInstructionsAllRows.begin(), ContributingInstructionsAllRows.end());
       }
     }
   }
@@ -672,7 +684,7 @@ void DxilViewIdState::CreateViewIdSets(const std::unordered_map<unsigned, Instru
     for (Instruction *pInst : itOut.second) {
       // Set output dependence on ViewId.
       if (DxilInst_ViewID VID = DxilInst_ViewID(pInst)) {
-        m_bUsesViewId = true;
+        DXASSERT(m_bUsesViewId, "otherwise, DxilModule flag not set properly");
         OutputsDependentOnViewId[outIdx] = true;
         continue;
       }

+ 8 - 5
lib/HLSL/ControlDependence.cpp

@@ -1,10 +1,14 @@
 ///////////////////////////////////////////////////////////////////////////////
 //                                                                           //
-// ViewIdAnalysis.cpp                                                        //
+// ControlDependence.cpp                                                     //
 // Copyright (C) Microsoft Corporation. All rights reserved.                 //
 // This file is distributed under the University of Illinois Open Source     //
 // License. See LICENSE.TXT for details.                                     //
 //                                                                           //
+// Control dependence is computed using algorithm in Figure 7.9 from [AK].   //
+//                                                                           //
+// References                                                                //
+// [AK] Optimizing Compilers for Modern Architectures by Allen and Kennedy.  //
 ///////////////////////////////////////////////////////////////////////////////
 
 #include "dxc/HLSL/ControlDependence.h"
@@ -67,11 +71,10 @@ void ControlDependence::Compute(Function *F, PostDomRelationType &PostDomRel) {
       }
     }
 
-    SmallVector<BasicBlock *, 8> Descendants;
-    PostDomRel.getDescendants(x, Descendants);
     // For all z such that ipostdom(z) = x
-    for (BasicBlock *z : Descendants) {
-      if (z == x) continue;
+    for (DomTreeNode *child : PostDomRel.getNode(x)->getChildren()) {
+      BasicBlock *z = child->getBlock();
+
       auto it = m_ControlDependence.find(z);
       if (it == m_ControlDependence.end())
         continue;

+ 1 - 0
lib/HLSL/DxcOptimizer.cpp

@@ -88,6 +88,7 @@ HRESULT SetupRegistryPassForHLSL() {
     initializeDxilEmitMetadataPass(Registry);
     initializeDxilExpandTrigIntrinsicsPass(Registry);
     initializeDxilForceEarlyZPass(Registry);
+    initializeDxilFinalizeModulePass(Registry);
     initializeDxilGenerationPassPass(Registry);
     initializeDxilLegalizeEvalOperationsPass(Registry);
     initializeDxilLegalizeResourceUsePassPass(Registry);

+ 1 - 0
lib/HLSL/DxilLinker.cpp

@@ -628,6 +628,7 @@ void DxilLinkJob::RunPreparePass(Module &M) {
   PM.add(createCFGSimplificationPass());
 
   PM.add(createDxilCondenseResourcesPass());
+  PM.add(createDxilFinalizeModulePass());
   PM.add(createComputeViewIdStatePass());
   PM.add(createDxilEmitMetadataPass());
 

+ 33 - 3
lib/HLSL/DxilPreparePasses.cpp

@@ -168,10 +168,10 @@ void CheckInBoundForTGSM(GlobalVariable &GV, const DataLayout &DL) {
   }
 }
 
-class DxilEmitMetadata : public ModulePass {
+class DxilFinalizeModule : public ModulePass {
 public:
   static char ID; // Pass identification, replacement for typeid
-  explicit DxilEmitMetadata() : ModulePass(ID) {}
+  explicit DxilFinalizeModule() : ModulePass(ID) {}
 
   const char *getPassName() const override { return "HLSL DXIL Metadata Emit"; }
 
@@ -348,7 +348,37 @@ public:
       DM.CollectShaderFlags(); // Update flags to reflect any changes.
                                // Update Validator Version
       DM.UpgradeToMinValidatorVersion();
-      DM.EmitDxilMetadata();
+      return true;
+    }
+
+    return false;
+  }
+};
+}
+
+char DxilFinalizeModule::ID = 0;
+
+ModulePass *llvm::createDxilFinalizeModulePass() {
+  return new DxilFinalizeModule();
+}
+
+INITIALIZE_PASS(DxilFinalizeModule, "hlsl-dxilfinalize", "HLSL DXIL Finalize Module", false, false)
+
+
+///////////////////////////////////////////////////////////////////////////////
+
+namespace {
+
+class DxilEmitMetadata : public ModulePass {
+public:
+  static char ID; // Pass identification, replacement for typeid
+  explicit DxilEmitMetadata() : ModulePass(ID) {}
+
+  const char *getPassName() const override { return "HLSL DXIL Metadata Emit"; }
+
+  bool runOnModule(Module &M) override {
+    if (M.HasDxilModule()) {
+      M.GetDxilModule().EmitDxilMetadata();
       return true;
     }
 

+ 2 - 0
lib/Transforms/IPO/PassManagerBuilder.cpp

@@ -293,6 +293,7 @@ void PassManagerBuilder::populateModulePassManager(
       MPM.add(createMultiDimArrayToOneDimArrayPass());// HLSL Change
       MPM.add(createDxilCondenseResourcesPass()); // HLSL Change
       MPM.add(createDxilLegalizeSampleOffsetPass()); // HLSL Change
+      MPM.add(createDxilFinalizeModulePass());      // HLSL Change
       MPM.add(createComputeViewIdStatePass());    // HLSL Change
       MPM.add(createDxilEmitMetadataPass());      // HLSL Change
     }
@@ -563,6 +564,7 @@ void PassManagerBuilder::populateModulePassManager(
     MPM.add(createDxilCondenseResourcesPass());
     if (DisableUnrollLoops)
       MPM.add(createDxilLegalizeSampleOffsetPass()); // HLSL Change
+    MPM.add(createDxilFinalizeModulePass());
     MPM.add(createComputeViewIdStatePass()); // HLSL Change
     MPM.add(createDxilEmitMetadataPass());
   }

+ 32 - 0
tools/clang/test/CodeGenHLSL/viewid/viewid19.hlsl

@@ -0,0 +1,32 @@
+// RUN: %dxc -E main -T vs_6_1 %s | FileCheck %s
+
+// CHECK: Number of inputs: 7, outputs: 7
+// CHECK: Outputs dependent on ViewId: { 4, 5, 6 }
+// CHECK: Inputs contributing to computation of Outputs:
+// CHECK:   output 0 depends on inputs: { 0 }
+// CHECK:   output 1 depends on inputs: { 1 }
+// CHECK:   output 2 depends on inputs: { 2 }
+
+struct VertexShaderInput
+{
+	float3 pos : POSITION;
+	float3 color : COLOR0;
+	uint viewID : SV_ViewID;
+};
+struct VertexShaderOutput
+{
+	float4 pos : SV_POSITION;
+	float3 color : COLOR0;
+};
+
+VertexShaderOutput main(VertexShaderInput input)
+{
+	VertexShaderOutput output;
+	output.pos = float4(input.pos, 1.0f);
+	if (input.viewID % 2) {
+		output.color = float3(1.0f, 0.0f, 1.0f);
+	} else {
+		output.color = float3(0.0f, 1.0f, 0.0f);
+	}
+	return output;
+}

+ 1 - 0
tools/clang/unittests/HLSL/CompilerTest.cpp

@@ -5406,6 +5406,7 @@ TEST_F(CompilerTest, ViewID) {
   CodeGenTestCheck(L"..\\CodeGenHLSL\\viewid\\viewid16.hlsl");
   CodeGenTestCheck(L"..\\CodeGenHLSL\\viewid\\viewid17.hlsl");
   CodeGenTestCheck(L"..\\CodeGenHLSL\\viewid\\viewid18.hlsl");
+  CodeGenTestCheck(L"..\\CodeGenHLSL\\viewid\\viewid19.hlsl");
 }
 
 TEST_F(CompilerTest, ShaderCompatSuite) {

+ 1 - 0
utils/hct/hctdb.py

@@ -1282,6 +1282,7 @@ class db_dxil(object):
         add_pass('hlsl-dxil-remove-discards', 'DxilRemoveDiscards', 'HLSL DXIL Remove all discard instructions', [])
         add_pass('hlsl-dxil-force-early-z', 'DxilForceEarlyZ', 'HLSL DXIL Force the early Z global flag, if shader has no discard calls', [])
         add_pass('hlsl-dxil-reduce-msaa-to-single', 'DxilReduceMSAAToSingleSample', 'HLSL DXIL Reduce all MSAA reads to single-sample reads', [])
+        add_pass('hlsl-dxilfinalize', 'DxilFinalizeModule', 'HLSL DXIL Finalize Module', [])
         add_pass('hlsl-dxilemit', 'DxilEmitMetadata', 'HLSL DXIL Metadata Emit', [])
         add_pass('hlsl-dxilload', 'DxilLoadMetadata', 'HLSL DXIL Metadata Load', [])
         add_pass('hlsl-dxil-expand-trig', 'DxilExpandTrigIntrinsics', 'DXIL expand trig intrinsics', [])