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

[Christudasan Devadasan via Phabricator via llvm-commits]

cdevadas marked 3 inline comments as done.
cdevadas 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;
----------------
arsenm wrote:
> 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
Yes. But retained this code for the existing mir tests added for SIInsertSkip. This file will go away entirely after handling the kill intrinsics and other unrelated implementations elsewhere. Once the new design is in the upstream, I will clean-up the mir tests too.


================
Comment at: lib/Target/AMDGPU/SIRemoveShortExecBranches.cpp:66
+
+bool SIRemoveShortExecBranches::getBranchTargets(
+    MachineBasicBlock &SrcMBB, MachineBasicBlock *&TrueMBB,
----------------
arsenm wrote:
> This name is slightly misleading. getBlockDestinations?
sure, I will use this name.


================
Comment at: lib/Target/AMDGPU/SIRemoveShortExecBranches.cpp:139
+  MF.RenumberBlocks();
+  bool Changed = false;
+
----------------
arsenm wrote:
> 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
Inserted the renumbering here to fix (restore) any broken BB numbering if any prior optimization pass has removed/added the BBs.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68092