[PATCH] D31350: AMDGPU : Fix common dominator of two incoming blocks terminates with uniform branch issue.

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: lib/Target/AMDGPU/SIFixSGPRCopies.cpp:333
+                                 const TargetRegisterInfo *TRI) {
+  DenseSet<MachineBasicBlock*> BBSet;
+  SmallVector<MachineBasicBlock*, 2> Svec;
----------------
Usually it is called Visited.


================
Comment at: lib/Target/AMDGPU/SIFixSGPRCopies.cpp:334
+  DenseSet<MachineBasicBlock*> BBSet;
+  SmallVector<MachineBasicBlock*, 2> Svec;
+  MachineBasicBlock *Seperator = nullptr;
----------------
And this is usually Worklist.


================
Comment at: lib/Target/AMDGPU/SIFixSGPRCopies.cpp:337
+  Svec.push_back(MBB);
+  Svec.push_back(Seperator);
+
----------------
Not needed.


================
Comment at: lib/Target/AMDGPU/SIFixSGPRCopies.cpp:339
+
+  while (Svec.size()) {
+    MachineBasicBlock *mbb = Svec.front();
----------------
SmallVector<MachineBasicBlock*, 4> Worklist(MBB->pred_begin(), MBB->pred_end());
while (!Worklist.empty())


================
Comment at: lib/Target/AMDGPU/SIFixSGPRCopies.cpp:341
+    MachineBasicBlock *mbb = Svec.front();
+    Svec.erase(Svec.begin());
+    if (mbb != Seperator) {
----------------
pop_back_val.


================
Comment at: lib/Target/AMDGPU/SIFixSGPRCopies.cpp:343
+    if (mbb != Seperator) {
+      if (BBSet.find(mbb) == BBSet.end()) {
+        if (hasTerminatorThatModifiesExec(*mbb, *TRI))
----------------
if (Visited.insert(mbb).second) continue;


================
Comment at: lib/Target/AMDGPU/SIFixSGPRCopies.cpp:348
+        BBSet.insert(mbb);
+        for (MachineBasicBlock *preds : mbb->predecessors()) {
+          Svec.push_back(preds);
----------------
pred, not preds. Anyway, Worklist.insert(Worklist.end(), mbb->pred_begin(), mbb->pred_end()).


Repository:
  rL LLVM

https://reviews.llvm.org/D31350