[PATCH] D68092: [AMDGPU] Invert the handling of skip insertion.

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/Target/AMDGPU/SIInsertSkips.cpp:469-475
+      case AMDGPU::S_CBRANCH_EXECZ:
+        ExecBranchStack.push_back(MI.getOperand(0).getMBB());
+        break;
       case AMDGPU::SI_MASK_BRANCH:
         ExecBranchStack.push_back(MI.getOperand(0).getMBB());
         MadeChange |= skipMaskBranch(MI, MBB);
         break;
----------------
Shouldn't this be dead code now? i.e. you shouldn't be adding a case, and should be removing the SI_MASK_BRANCH part


================
Comment at: lib/Target/AMDGPU/SIRemoveShortExecBranches.cpp:66
+
+bool SIRemoveShortExecBranches::getBranchTargets(
+    MachineBasicBlock &SrcMBB, MachineBasicBlock *&TrueMBB,
----------------
This name is slightly misleading. getBlockDestinations?


================
Comment at: lib/Target/AMDGPU/SIRemoveShortExecBranches.cpp:139
+  MF.RenumberBlocks();
+  bool Changed = false;
+
----------------
I'm not sure if potentially renumbering the blocks counts as a change, but it probably doesn't really matter. It's not a guaranteed property between passes I guess


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68092