Infer wave-sensitivity of cyclic dependencies (#3543)

Cycles in the dependency graph can result in phis and other instructions
failing to get a wave sensitivity result after analysis. This can only
happen when nothing in the loop has wave sensitive operations.

By collecting phis that are unknown and marking them as non-sensitive
after analyze is complete and rerunning it, we can correctly evaluate
their dependants too.

Because incompletely processed blocks might still be wave-sensitive,
unknown phis are only added to be reprocessed when their preds have all
be processed fully.

In addition to cycles preventing wave sensitivity determination on
dependant operations, they can prevent entering blocks that depend on
values with cyclic dependencies. Even when reprocessing all unknown
phis, this can result in not all values getting a determination.

To resolve this, all blocks are added to the block list from the start
to ensure that they are all processed eventually. Since instruction
processing is more intelligent and targetted, this change is accompanied
by a change that only processes a single block from the worklist before
attempting to address queued instructions again.

To catch wave-sensitive instructions sooner, when visiting an
instruction, all arguments are checked for wave sensitivity even after
an unknown argument is found whereas before an unknown argument ended
the search.

Add some tricky loop tests that failed in ToT and various intermediate
stages of the change. Add a more aggressive assert to check for failures
to find the wave-sensitivity.

Fixed #2556 (again)

lib/HLSL/WaveSensitivityAnalysis.cpp

@@ -63,6 +63,7 @@ private:
   map<Instruction *, WaveSensitivity> InstState;
   map<BasicBlock *, WaveSensitivity> BBState;
   std::vector<Instruction *> InstWorkList;
+  std::vector<PHINode *> UnknownPhis; // currently unknown phis. Indicate cycles after Analyze
   std::vector<BasicBlock *> BBWorkList;
   bool CheckBBState(BasicBlock *BB, WaveSensitivity WS);
   WaveSensitivity GetInstState(Instruction *I);
@@ -72,6 +73,7 @@ private:
 public:
   WaveSensitivityAnalyzer(PostDominatorTree &PDT) : pPDT(&PDT) {}
   void Analyze(Function *F);
+  void Analyze();
   bool IsWaveSensitive(Instruction *op);
 };
 
@@ -79,8 +81,58 @@ WaveSensitivityAnalysis* WaveSensitivityAnalysis::create(PostDominatorTree &PDT)
   return new WaveSensitivityAnalyzer(PDT);
 }
 
+// Analyze the given function's instructions as wave-sensitive or not
 void WaveSensitivityAnalyzer::Analyze(Function *F) {
-  UpdateBlock(&F->getEntryBlock(), KnownNotSensitive);
+  // Add all blocks but the entry in reverse order so they come out in order
+  auto it = F->getBasicBlockList().end();
+  for ( it-- ; it != F->getBasicBlockList().begin(); it--)
+    BBWorkList.emplace_back(&*it);
+  // Add entry block as non-sensitive
+  UpdateBlock(&*it, KnownNotSensitive);
+
+  // First analysis
+  Analyze();
+
+  // If any phis with explored preds remain unknown
+  // it has to be in a loop that don't include wave sensitivity
+  // Update each as such and redo Analyze to mark the descendents
+  while (!UnknownPhis.empty() || !InstWorkList.empty() || !BBWorkList.empty()) {
+    while (!UnknownPhis.empty()) {
+      PHINode *Phi = UnknownPhis.back();
+      UnknownPhis.pop_back();
+      // UnknownPhis might have actually known phis that were changed. skip them
+      if (Unknown == GetInstState(Phi)) {
+        // If any of the preds have not been visited, we can't assume a cycle yet
+        bool allPredsVisited = true;
+        for (unsigned i = 0; i < Phi->getNumIncomingValues(); i++) {
+          if (!BBState.count(Phi->getIncomingBlock(i))) {
+            allPredsVisited = false;
+            break;
+          }
+        }
+#ifndef NDEBUG
+        for (User *U : Phi->users()) {
+          Instruction *UI = cast<Instruction>(U);
+          DXASSERT(GetInstState(UI) != KnownSensitive, "Unknown wave-status Phi argument should not be able to be known sensitive");
+        }
+#endif
+        if (allPredsVisited)
+          UpdateInst(Phi, KnownNotSensitive);
+      }
+    }
+    Analyze();
+  }
+#ifndef NDEBUG
+  for (BasicBlock &BB : *F) {
+    for (Instruction &I : BB) {
+      DXASSERT(Unknown != GetInstState(&I), "Wave sensitivity analysis exited without finding results for all instructions");
+    }
+  }
+#endif
+}
+
+// Analyze the member instruction and BBlock worklists
+void WaveSensitivityAnalyzer::Analyze() {
   while (!InstWorkList.empty() || !BBWorkList.empty()) {
     // Process the instruction work list.
     while (!InstWorkList.empty()) {
@@ -94,8 +146,8 @@ void WaveSensitivityAnalyzer::Analyze(Function *F) {
       }
     }
 
-    // Process the basic block work list.
-    while (!BBWorkList.empty()) {
+    // Process one entry of the basic block work list.
+    if (!BBWorkList.empty()) {
       BasicBlock *BB = BBWorkList.back();
       BBWorkList.pop_back();
 
@@ -184,6 +236,8 @@ void WaveSensitivityAnalyzer::VisitInst(Instruction *I) {
       if (WS == KnownSensitive) {
         UpdateInst(I, KnownSensitive);
         return;
+      } else if (Unknown == GetInstState(I)) {
+        UnknownPhis.emplace_back(Phi);
       }
     }
   }
@@ -196,10 +250,8 @@ void WaveSensitivityAnalyzer::VisitInst(Instruction *I) {
       if (WS == KnownSensitive) {
         UpdateInst(I, KnownSensitive);
         return;
-      }
-      if (WS == Unknown) {
+      } else if (WS == Unknown) {
         allKnownNotSensitive = false;
-        return;
       }
     }
   }

tools/clang/test/HLSLFileCheck/hlsl/intrinsics/wave/reduction/WaveSensitivityAndCycles.hlsl

@@ -0,0 +1,55 @@
+// RUN: %dxc -T lib_6_3 %s | FileCheck %s -input=stderr
+
+// CHECK: warning: Gradient operations are not affected by wave-sensitive data or control flow
+// CHECK: warning: Gradient operations are not affected by wave-sensitive data or control flow
+// CHECK: warning: Gradient operations are not affected by wave-sensitive data or control flow
+
+export
+float main1(float dep : DEPTH) : SV_TARGET {
+  // Loop creates cycle in dependency graph
+  while( dep < 100.0 )
+    dep += 1.0;
+  // wave-sensitive and gradiant ops to prevent disabling sensitivity search
+  float wave = QuadReadLaneAt( dep, 0 );
+  float grad = ddx( dep );
+
+  // Gradiant that depends on wave op
+  return ddy(wave + grad);
+}
+
+export
+float main2(float dep0 : DEPTH0, float dep1 : DEPTH0) : SV_TARGET {
+  // Double loop creates cycle in dependency graph
+  // and also phis at the start that depend on blocks with branches
+  // that depend on cycle dependant values to get entered
+  while( dep0 < 100.0 ) {
+    while( dep1 < 100.0 ) {
+      dep0 += 1.0;
+      dep1 += 1.0;
+    }
+  }
+  // wave-sensitive and gradiant ops to prevent disabling sensitivity search
+  float wave = QuadReadLaneAt( dep1, 0 );
+  float grad = ddx( dep0 );
+
+  // Gradiant that dep0ends on wave op to produce warning
+  return ddy(wave + grad);
+}
+
+export
+float main3(float dep0 : DEPTH0, float dep1 : DEPTH0) : SV_TARGET {
+  // Double loop creates cycle in dependency graph
+  // and also phis at the start that depend on blocks with branches
+  // that depend on cycle dependant values to get entered
+  while( dep0 < 100.0 ) {
+    while( dep1 < 100.0 ) {
+      // Backward dependant Gradiant that depends on wave op
+      // These are in a block that was entirely unvisited previously
+      dep0 += ddx(dep0);
+      dep1 += ddy(dep1);
+    }
+    dep0 = QuadReadLaneAt( dep0, 0);
+    dep1 = QuadReadLaneAt( dep1, 0);
+  }
+  return dep0 + dep1;
+}