Enable PropagateCmpUniformity pass by default

This pass improves scalarization performance by propagating uniform values
through conditional branches. The optimization is applied to non-RT shaders.

Bug fixes in some special cases.

Author: iwwu

IGC/Compiler/CISACodeGen/PropagateCmpUniformity.cpp

@@ -28,6 +28,7 @@ using namespace IGC;
 IGC_INITIALIZE_PASS_BEGIN(PropagateCmpUniformity, PASS_FLAG, PASS_DESCRIPTION, PASS_CFG_ONLY, PASS_ANALYSIS)
 IGC_INITIALIZE_PASS_DEPENDENCY(WIAnalysis)
 IGC_INITIALIZE_PASS_DEPENDENCY(CodeGenContextWrapper)
+IGC_INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
 IGC_INITIALIZE_PASS_END(PropagateCmpUniformity, PASS_FLAG, PASS_DESCRIPTION, PASS_CFG_ONLY, PASS_ANALYSIS)
 
 char PropagateCmpUniformity::ID = 0;
@@ -125,50 +126,71 @@ bool PropagateCmpUniformity::getUniformNonUniformPair(Value *op0, Value *op1, Va
   }
 }
 
-bool PropagateCmpUniformity::canReplaceUse(Use &U, BasicBlock *trueBranch, BasicBlock *cmpBB, DominatorTree &DT) {
+bool PropagateCmpUniformity::canReplaceUse(Use &U, BasicBlock *trueBranchBB, BasicBlock *cmpBB, DominatorTree &DT) {
   User *user = U.getUser();
   BasicBlock *useBB = nullptr;
   BasicBlock *incomingBB = nullptr;
 
   if (PHINode *phi = dyn_cast<PHINode>(user)) {
     useBB = phi->getParent();
     incomingBB = phi->getIncomingBlock(U);
-
-    // Special case: PHI in trueBranch with incoming edge directly from cmpBB.
-    // Equality holds on the cmpBB trueBranch edge, so replacing the incoming
-    // nonUniform value with uniform is valid — but only when falseBranch (the
-    // other successor of cmpBB) is NOT a direct predecessor of trueBranch.
-    // If falseBranch also feeds trueBranch, CFGSimplification may later merge
-    // an empty falseBranch into cmpBB, producing two incoming edges from the
-    // same block and collapsing the PHI incorrectly (e.g. to a zero constant).
-    if (incomingBB == cmpBB && useBB == trueBranch) {
-      BranchInst *cmpBr = cast<BranchInst>(cmpBB->getTerminator());
-      BasicBlock *falseBranch =
-          (cmpBr->getSuccessor(0) == trueBranch) ? cmpBr->getSuccessor(1) : cmpBr->getSuccessor(0);
-      for (BasicBlock *pred : predecessors(trueBranch))
-        if (pred == falseBranch)
-          return false;
-      return true;
-    }
-
   } else if (Instruction *inst = dyn_cast<Instruction>(user)) {
     useBB = inst->getParent();
     incomingBB = useBB;
   } else {
     return false;
   }
 
-  // trueBranch must have cmpBB as its only predecessor. If trueBranch is a
-  // join point with multiple predecessors, it is reachable without going
-  // through the equality-proven edge, so the equality may not hold on all
-  // paths to the use even if trueBranch dominates it.
-  if (trueBranch->getSinglePredecessor() != cmpBB)
+  // Compute falseBranchBB once for PHI uses; nullptr for non-PHI.
+  BasicBlock *falseBranchBB = nullptr;
+  if (isa<PHINode>(user)) {
+    BranchInst *cmpBr = cast<BranchInst>(cmpBB->getTerminator());
+    falseBranchBB = (cmpBr->getSuccessor(0) == trueBranchBB) ? cmpBr->getSuccessor(1) : cmpBr->getSuccessor(0);
+  }
+
+  // Special case: PHI in trueBranchBB with incoming edge directly from cmpBB.
+  // Equality holds on cmpBB->trueBranchBB, so the replacement is valid — unless
+  // falseBranchBB can also reach trueBranchBB, either directly or via intermediate
+  // blocks (e.g. created by JumpThreading). In that case a later CFGSimplification
+  // would collapse the intermediate blocks back and corrupt the PHI.
+  if (incomingBB == cmpBB && useBB == trueBranchBB) {
+    // Sub-case: cmpBB's two successors are the same block (both edges to trueBranchBB).
+    if (falseBranchBB == trueBranchBB)
+      return false;
+    // Sub-case: falseBranchBB is a direct predecessor of trueBranchBB.
+    for (BasicBlock *pred : predecessors(trueBranchBB))
+      if (pred == falseBranchBB)
+        return false;
+    // Sub-case: falseBranchBB reaches trueBranchBB via intermediate blocks that are
+    // dominated by cmpBB (i.e. they lie on the false-branch path). After a later
+    // CFGSimplification those intermediates would be merged away, corrupting the PHI.
+    for (BasicBlock *pred : predecessors(trueBranchBB))
+      if (pred != cmpBB && DT.dominates(cmpBB, pred))
+        return false;
+    return true;
+  }
+
+  // trueBranchBB must have cmpBB as its only predecessor; otherwise equality may
+  // not hold on all paths to the use.
+  if (trueBranchBB->getSinglePredecessor() != cmpBB)
     return false;
 
-  // Only replace if trueBranch dominates the use's incoming BB.
+  // For PHI uses: reject if falseBranchBB or cmpBB is also a direct predecessor
+  // of useBB. CFGSimplification would later collapse those edges back to cmpBB,
+  // overwriting the non-equality incoming value with the uniform constant.
+  // (The cmpBB check covers the case where an earlier CFGSimplification pass
+  // already merged trueBranchBB away, leaving cmpBB with a direct false edge to
+  // useBB where the equality guarantee does not hold.)
+  if (falseBranchBB) {
+    for (BasicBlock *pred : predecessors(useBB))
+      if (pred == falseBranchBB || pred == cmpBB)
+        return false;
+  }
+
+  // Only replace if trueBranchBB dominates the use's incomingBB.
   // Together with the single-predecessor check above, this guarantees every
-  // path to the use went through the equality-proven cmpBB→trueBranch edge.
-  return DT.dominates(trueBranch, incomingBB);
+  // path to the use went through the equality-proven cmpBB->trueBranchBB edge.
+  return DT.dominates(trueBranchBB, incomingBB);
 }
 
 bool PropagateCmpUniformity::replaceNonUniformWithUniform(CmpInst *cmp, Value *nonUniform, Value *uniform,

IGC/Compiler/CISACodeGen/ShaderCodeGen.cpp

@@ -1614,7 +1614,10 @@ void OptimizeIR(CodeGenContext *const pContext) {
         mpm.add(llvm::createJumpThreadingPass(false, BBDuplicateThreshold));
 #endif // LLVM_VERSION_MAJOR
       }
-      if (IGC_IS_FLAG_ENABLED(EnablePropagateCmpUniformity)) {
+      // RT shaders have many short equality branches (dispatch patterns)
+      // where live range extension costs outweigh scalarization benefits at SIMD8,
+      // and FunctionMultiversioning is disabled for RT (no compounding benefit).
+      if (IGC_IS_FLAG_ENABLED(EnablePropagateCmpUniformity) && pContext->type != ShaderType::RAYTRACING_SHADER) {
         mpm.add(createPropagateCmpUniformityPass());
       }
       mpm.add(llvm::createCFGSimplificationPass());

IGC/Compiler/tests/PropagateCmpUniformity/cmpUniform.ll

@@ -478,15 +478,113 @@ exit:
   ret void
 }
 
+; ============================================================================
+; Test 15: falseBranch is also a direct predecessor of the PHI's parent block
+;          (useBB) — no replacement allowed.
+;
+; CFG produced by SROA when a dead ternary alloca is promoted:
+;
+;   cmpBB: fcmp oeq %val, 0.0
+;     - ternary.true  (trueBranch, empty: br endBB)
+;     - ternary.false (falseBranch, empty: br endBB)
+;                          |
+;                          V
+;                        endBB: phi [%val, ternary.true], [%val, ternary.false]
+;
+; Must NOT replace %val in the [%val, %ternary.true] incoming because
+; falseBranch is also a direct predecessor of endBB (useBB).  If it did,
+; CFGSimplification would later remove both empty blocks, collapsing the
+; two incoming edges back to cmpBB and discarding the ternary.false value,
+; turning the PHI into the constant 0.0.  This caused incorrect rendering in
+; a GS shader where the diagonal faceNormal components were zeroed out.
+; ============================================================================
+; CHECK-LABEL: @test_falsebranch_also_pred_of_usebb(
+define spir_kernel void @test_falsebranch_also_pred_of_usebb(ptr addrspace(1) %out) {
+entry:
+  %tid = call i32 @llvm.genx.GenISA.DCL.SystemValue.i32(i32 17)
+  %val = uitofp i32 %tid to float
+  %cmp = fcmp oeq float %val, 0.000000e+00
+  br i1 %cmp, label %ternary.true, label %ternary.false
+
+ternary.true:
+  br label %endBB
+
+ternary.false:
+  br label %endBB
+
+endBB:
+  ; falseBranch (%ternary.false) is a direct predecessor of endBB, so
+  ; the [%val, %ternary.true] arm must not be replaced with 0.0.
+; CHECK-LABEL: endBB:
+; CHECK: %phi_val = phi float [ %val, %ternary.true ], [ %val, %ternary.false ]
+  %phi_val = phi float [ %val, %ternary.true ], [ %val, %ternary.false ]
+  store float %phi_val, ptr addrspace(1) %out, align 4
+  ret void
+}
+
+; ============================================================================
+; Test 16: falseBranch reaches trueBranchBB (useBB) via an intermediate block
+;          that is dominated by cmpBB — no replacement allowed.
+;
+; This models the CFG produced by JumpThreading when it threads the true path of
+; a ternary-style conditional directly to the downstream join block (endBB),
+; while the false path still goes through intermediate blocks:
+;
+;   cmpBB: fcmp oeq %val, 0.0
+;     true  ------------------------------> endBB
+;     false --> ternary.false --> ternary.end --> endBB
+;
+; PCU's Check 1 fires: incomingBB==cmpBB and useBB==trueBranchBB==endBB.
+; The direct-predecessor guard (sub-case A, Test 15) does not help because
+; ternary.false is NOT a direct predecessor of endBB.  The new guard catches
+; it: ternary.end is a predecessor of endBB, is not cmpBB, and IS dominated
+; by cmpBB — indicating it lies on the false-branch path.
+;
+; Must NOT replace %val in [%val, %entry] because ternary.end reaches endBB
+; without the equality guarantee, so after CFGSimplification collapses
+; ternary.false->ternary.end the PHI would incorrectly receive the constant 0.0.
+; This was the root cause of a GS shader rendering bug (IGC-13513) where the
+; diagonal faceNormal component was zeroed out.
+; ============================================================================
+; CHECK-LABEL: @test_indirect_falsebranch(
+define spir_kernel void @test_indirect_falsebranch(ptr addrspace(1) %out) {
+entry:
+  %tid = call i32 @llvm.genx.GenISA.DCL.SystemValue.i32(i32 17)
+  %val = uitofp i32 %tid to float
+  %cmp = fcmp oeq float %val, 0.000000e+00
+  ; True edge goes directly to endBB (as if JumpThreading threaded ternary.true away).
+  ; False edge goes to ternary.false, which eventually also reaches endBB.
+  br i1 %cmp, label %endBB, label %ternary.false
+
+ternary.false:
+  ; Intermediate block dominated by entry (=cmpBB); no equality guarantee here.
+  br label %ternary.end
+
+ternary.end:
+  ; Also dominated by entry (=cmpBB); direct predecessor of endBB.
+  br label %endBB
+
+endBB:
+  ; [%val, %entry]    — incoming from cmpBB on the true (equality) edge.
+  ; [%val, %ternary.end] — incoming from the false-path intermediate block.
+  ; The [%val, %entry] arm must NOT be replaced with 0.0: ternary.end reaches
+  ; here without equality being guaranteed.
+; CHECK-LABEL: endBB:
+; CHECK: %phi_val = phi float [ %val, %entry ], [ %val, %ternary.end ]
+  %phi_val = phi float [ %val, %entry ], [ %val, %ternary.end ]
+  store float %phi_val, ptr addrspace(1) %out, align 4
+  ret void
+}
+
 declare i32 @llvm.genx.GenISA.DCL.SystemValue.i32(i32) #0
 
 attributes #0 = { nounwind readnone }
 
 !IGCMetadata = !{!2}
-!igc.functions = !{!13, !22, !32, !42, !52, !62, !72, !82, !92, !102, !112, !122, !132, !142}
+!igc.functions = !{!13, !22, !32, !42, !52, !62, !72, !82, !92, !102, !112, !122, !132, !142, !152, !162}
 
 !2 = !{!"ModuleMD", !3}
-!3 = !{!"FuncMD", !4, !5, !20, !21, !30, !31, !40, !41, !50, !51, !60, !61, !70, !71, !80, !81, !90, !91, !100, !101, !110, !111, !120, !121, !130, !131, !140, !141}
+!3 = !{!"FuncMD", !4, !5, !20, !21, !30, !31, !40, !41, !50, !51, !60, !61, !70, !71, !80, !81, !90, !91, !100, !101, !110, !111, !120, !121, !130, !131, !140, !141, !150, !151, !160, !161}
 
 ; Shared metadata nodes reused by all kernels
 !6  = !{!"localOffsets"}
@@ -569,3 +667,13 @@ attributes #0 = { nounwind readnone }
 !141 = !{!"FuncMDValue[13]", !6, !7, !11, !12}
 !142 = !{ptr @test_multi_cmpbb_truebb, !14}
 
+; test_falsebranch_also_pred_of_usebb
+!150 = !{!"FuncMDMap[14]", ptr @test_falsebranch_also_pred_of_usebb}
+!151 = !{!"FuncMDValue[14]", !6, !7, !11, !12}
+!152 = !{ptr @test_falsebranch_also_pred_of_usebb, !14}
+
+; test_indirect_falsebranch
+!160 = !{!"FuncMDMap[15]", ptr @test_indirect_falsebranch}
+!161 = !{!"FuncMDValue[15]", !6, !7, !11, !12}
+!162 = !{ptr @test_indirect_falsebranch, !14}
+

IGC/common/igc_flags.h

@@ -544,7 +544,7 @@ DECLARE_IGC_REGKEY(bool, DisableIPConstantPropagation, false, "Disable Inter-pro
 DECLARE_IGC_REGKEY(bool, EnableSplitIndirectEEtoSel, true, "Enable the split indirect extractelement to icmp+sel pass",
                    false)
 DECLARE_IGC_REGKEY(DWORD, SplitIndirectEEtoSelThreshold, 8, "Split indirect extractelement cost threshold", false)
-DECLARE_IGC_REGKEY(DWORD, EnablePropagateCmpUniformity, 0,
+DECLARE_IGC_REGKEY(DWORD, EnablePropagateCmpUniformity, 1,
                    "Enable propagation of compare-based uniformity: replace non-uniform/divergent values with uniform "
                    "values in dominated BBs as determined by WIAnalysis (1 enable, 2 enable and print)",
                    false)