[PATCH] D130746: RFC: Uniformity Analysis for Irreducible Control Flow

[Juan Manuel Martinez Caamaño via Phabricator via llvm-commits]

jmmartinez added inline comments.


================
Comment at: llvm/include/llvm/ADT/GenericUniformityImpl.h:944-945
+    // condition is placed in the worklist.
+    if (I.isTerminator())
+      break;
+
----------------
If I understand correctly, a MachineBasicBlock may have multiple terminators. So why stopping at the first terminator, instead of continuing ?


================
Comment at: llvm/include/llvm/ADT/GenericUniformityImpl.h:973-978
+  for (auto *C : Cycles) {
+    if (C == Candidate)
+      return false;
+    if (C->contains(Candidate))
+      return false;
+  }
----------------
`C->contains(Candidate)` already checks if `C == Candidate`

What do you think about rewritting this loop as an `any_of`?

Also, I would assume that it is an outermost cycle if its depth is 1. Wouldn't be better to check this instead of it the cycle is contained by another? or am I missing something?


================
Comment at: llvm/lib/IR/SSAContext.cpp:42
+      continue;
+    auto *def = &instr;
+    defs.push_back(def);
----------------
Is it normal that values with `void` type are included among the definitions ? I understand that this is intentional but just wanted to confirm.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130746/new/

https://reviews.llvm.org/D130746