[LLVM][Uniformity] Propagate divergence only along divergent outputs.

[sameerds]

Summary

When an instruction is determined to be divergent, not all its outputs are
divergent. The users of only divergent outputs should now be examined for
divergence.

Also, replaced a repeating pattern of "if new divergent instruction, then add to
worklist" by combining it into a single function. This does not cause any change
in functionality.

[foad]

LGTM with or without nits addressed.

llvm/include/llvm/ADT/GenericUniformityImpl.h

358–359

Maybe it's time to comment each function separately? The one you've changed no longer returns anything.

786–788

else after return looks odd and it seems easy to avoid it here.

[sameerds]

• added a small but important fix
• eliminated return-after-else

[sameerds]

Requesting a fresh review because I added a fix. markDefsDivergent() only records divergent outputs of a multi-output instruction, but pushUsers() was blindly propagating divergence along the uses of the uniform outputs. The change was small and related to what I was doing, so didn't create a separate review. This eliminates a FIXME in an existing MIR test.

[foad]

llvm/test/Analysis/UniformityAnalysis/AMDGPU/MIR/always-uniform-gmir.mir

100 ↗

(On Diff #522501)

Just checking my understanding: this should have said "FIXME: first output of below inline asm should be uniform"?

[sameerds]

llvm/test/Analysis/UniformityAnalysis/AMDGPU/MIR/always-uniform-gmir.mir

100 ↗

(On Diff #522501)

That is my understanding too! After recent changes, it is certainly true that the s32 output should be uniform, and this current change ensures that its use in the COPY instruction also remains uniform. If I revert the current change but keep this test, then the CHECK-NOT fails for %4.

llvm/include/llvm/ADT/GenericUniformityImpl.h

public:
 
   void initialize();
 
   const FunctionT &getFunction() const { return F; }
 
   /// \brief Mark \p UniVal as a value that is always uniform.
   void addUniformOverride(const InstructionT &Instr);
 
   /// \brief Mark \p DivVal as a value that is always divergent.
   /// \returns Whether the tracked divergence state of \p DivVal changed.
-  bool markDivergent(const InstructionT &I);
+  void markDivergent(const InstructionT &I);
   bool markDivergent(ConstValueRefT DivVal);
   bool markDefsDivergent(const InstructionT &Instr);
 
   /// \brief Propagate divergence to all instructions in the region.
   /// Divergence is seeded by calls to \p markDivergent.
   void compute();
 
   /// \brief Whether any value was marked or analyzed to be divergent.
auto llvm::GenericSyncDependenceAnalysis<ContextT>::getJoinBlocks(
 
   auto ItInserted =
       CachedControlDivDescs.try_emplace(DivTermBlock, std::move(DivDesc));
   assert(ItInserted.second);
   return *ItInserted.first->second;
 }
 
 template <typename ContextT>
-bool GenericUniformityAnalysisImpl<ContextT>::markDivergent(
+void GenericUniformityAnalysisImpl<ContextT>::markDivergent(
     const InstructionT &I) {
+  bool Marked = false;
   if (I.isTerminator()) {
     if (DivergentTermBlocks.insert(I.getParent()).second) {
       LLVM_DEBUG(dbgs() << "marked divergent term block: "
                         << Context.print(I.getParent()) << "\n");
-      return true;
+      Marked = true;
     }
-    return false;
+  } else if (isAlwaysUniform(I)) {
+    return;
+  } else {
+    Marked = markDefsDivergent(I);
   }
 
-  if (isAlwaysUniform(I))
-    return false;
-
-  return markDefsDivergent(I);
+  if (Marked)
+    Worklist.push_back(&I);
 }
 
 template <typename ContextT>
 bool GenericUniformityAnalysisImpl<ContextT>::markDivergent(
     ConstValueRefT Val) {
   if (DivergentValues.insert(Val).second) {
     LLVM_DEBUG(dbgs() << "marked divergent: " << Context.print(Val) << "\n");
     return true;

 template <typename ContextT>
 void GenericUniformityAnalysisImpl<ContextT>::analyzeCycleExitDivergence(
     const CycleT &DefCycle) {
   SmallVector<BlockT *> Exits;
   DefCycle.getExitBlocks(Exits);
   for (auto *Exit : Exits) {
     for (auto &Phi : Exit->phis()) {
       if (usesValueFromCycle(Phi, DefCycle)) {
-        if (markDivergent(Phi))
-          Worklist.push_back(&Phi);
+        markDivergent(Phi);
       }
     }
   }
 
   for (auto *BB : DefCycle.blocks()) {
     if (!llvm::any_of(Exits,
                      [&](BlockT *Exit) { return DT.dominates(BB, Exit); }))
       continue;
void GenericUniformityAnalysisImpl<ContextT>::taintAndPushAllDefs(
   LLVM_DEBUG(dbgs() << "taintAndPushAllDefs " << Context.print(&BB) << "\n");
   for (const auto &I : instrs(BB)) {
     // Terminators do not produce values; they are divergent only if
     // the condition is divergent. That is handled when the divergent
     // condition is placed in the worklist.
     if (I.isTerminator())
       break;
 
-    if (markDivergent(I))
-      Worklist.push_back(&I);
+    markDivergent(I);
   }
 }
 
 /// Mark divergent phi nodes in a join block
 template <typename ContextT>
 void GenericUniformityAnalysisImpl<ContextT>::taintAndPushPhiNodes(
     const BlockT &JoinBlock) {
   LLVM_DEBUG(dbgs() << "taintAndPushPhiNodes in " << Context.print(&JoinBlock)
                     << "\n");
   for (const auto &Phi : JoinBlock.phis()) {
     // FIXME: The non-undef value is not constant per se; it just happens to be
     // uniform and may not dominate this PHI. So assuming that the same value
     // reaches along all incoming edges may itself be undefined behaviour. This
     // particular interpretation of the undef value was added to
     // DivergenceAnalysis in the following review:
     //
     // https://reviews.llvm.org/D19013
     if (ContextT::isConstantOrUndefValuePhi(Phi))
       continue;
-    if (markDivergent(Phi))
-      Worklist.push_back(&Phi);
+    markDivergent(Phi);
   }
 }
 
 /// Add \p Candidate to \p Cycles if it is not already contained in \p Cycles.
 ///
 /// \return true iff \p Candidate was added to \p Cycles.
 template <typename CycleT>
 static bool insertIfNotContained(SmallVector<CycleT *> &Cycles,

llvm/lib/Analysis/UniformityAnalysis.cpp

 bool llvm::GenericUniformityAnalysisImpl<SSAContext>::hasDivergentDefs(
     const Instruction &I) const {
   return isDivergent((const Value *)&I);
 }
 
 template <>
 bool llvm::GenericUniformityAnalysisImpl<SSAContext>::markDefsDivergent(
     const Instruction &Instr) {
-  return markDivergent(&Instr);
+  return markDivergent(cast<Value>(&Instr));
 }
 
 template <> void llvm::GenericUniformityAnalysisImpl<SSAContext>::initialize() {
   for (auto &I : instructions(F)) {
     if (TTI->isSourceOfDivergence(&I))
       markDivergent(I);
     else if (TTI->isAlwaysUniform(&I))
       addUniformOverride(I);
   }
   for (auto &Arg : F.args()) {
     if (TTI->isSourceOfDivergence(&Arg)) {
       markDivergent(&Arg);
     }
   }
 }
 
 template <>
 void llvm::GenericUniformityAnalysisImpl<SSAContext>::pushUsers(
     const Value *V) {
   for (const auto *User : V->users()) {
     if (const auto *UserInstr = dyn_cast<const Instruction>(User)) {
-      if (markDivergent(*UserInstr)) {
-        Worklist.push_back(UserInstr);
-      }
+      markDivergent(*UserInstr);
     }
   }
 }
 
 template <>
 void llvm::GenericUniformityAnalysisImpl<SSAContext>::pushUsers(
     const Instruction &Instr) {
   assert(!isAlwaysUniform(Instr));
void llvm::GenericUniformityAnalysisImpl<
     SSAContext>::propagateTemporalDivergence(const Instruction &I,
                                              const Cycle &DefCycle) {
   if (isDivergent(I))
     return;
   for (auto *User : I.users()) {
     auto *UserInstr = cast<Instruction>(User);
     if (DefCycle.contains(UserInstr->getParent()))
       continue;
-    if (markDivergent(*UserInstr))
-      Worklist.push_back(UserInstr);
+    markDivergent(*UserInstr);
   }
 }
 
 template <>
 bool llvm::GenericUniformityAnalysisImpl<SSAContext>::isDivergentUse(
     const Use &U) const {
   const auto *V = U.get();
   if (isDivergent(V))

llvm/lib/CodeGen/MachineUniformityAnalysis.cpp

for (const MachineBasicBlock &block : F) {
     for (const MachineInstr &instr : block) {
       auto uniformity = InstrInfo.getInstructionUniformity(instr);
       if (uniformity == InstructionUniformity::AlwaysUniform) {
         addUniformOverride(instr);
         continue;
       }
 
       if (uniformity == InstructionUniformity::NeverUniform) {
-        if (markDivergent(instr))
-          Worklist.push_back(&instr);
+        markDivergent(instr);
       }
     }
   }
 }
 
 template <>
 void llvm::GenericUniformityAnalysisImpl<MachineSSAContext>::pushUsers(
     Register Reg) {
   const auto &RegInfo = F.getRegInfo();
   for (MachineInstr &UserInstr : RegInfo.use_instructions(Reg)) {
-    if (markDivergent(UserInstr))
-      Worklist.push_back(&UserInstr);
+    markDivergent(UserInstr);
   }
 }
 
 template <>
 void llvm::GenericUniformityAnalysisImpl<MachineSSAContext>::pushUsers(
     const MachineInstr &Instr) {
   assert(!isAlwaysUniform(Instr));
   if (Instr.isTerminator())
for (auto &Op : I.operands()) {
     if (!Op.getReg().isVirtual())
       continue;
     auto Reg = Op.getReg();
     if (isDivergent(Reg))
       continue;
     for (MachineInstr &UserInstr : RegInfo.use_instructions(Reg)) {
       if (DefCycle.contains(UserInstr.getParent()))
         continue;
-      if (markDivergent(UserInstr))
-        Worklist.push_back(&UserInstr);
+      markDivergent(UserInstr);
     }
   }
 }
 
 template <>
 bool llvm::GenericUniformityAnalysisImpl<MachineSSAContext>::isDivergentUse(
     const MachineOperand &U) const {
   if (!U.isReg())