[PATCH] D94748: [AMDGPU] Tidy up conditional branches from early termination

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

I'm not sure this is the right pass for this (I've been hoping to move everything out of this pass eventually)



================
Comment at: llvm/lib/Target/AMDGPU/SIInsertSkips.cpp:195
 
+bool SIInsertSkips::tidySCCDef(MachineInstr &MI) {
+  MachineBasicBlock &MBB = *MI.getParent();
----------------
Comment what the point of this is


================
Comment at: llvm/lib/Target/AMDGPU/SIInsertSkips.cpp:201-202
+  auto Prev = std::prev(MI.getIterator());
+  if (Prev->getOpcode() == AMDGPU::S_ANDN2_B32 ||
+      Prev->getOpcode() == AMDGPU::S_ANDN2_B64) {
+    auto Src0 = Prev->getOperand(1);
----------------
Invert and early return?


================
Comment at: llvm/lib/Target/AMDGPU/SIInsertSkips.cpp:205-206
+    auto Src1 = Prev->getOperand(2);
+    if (Src0.isReg() && Src0.getReg() == ExecReg && Src1.isReg() &&
+        Src1.getReg() == ExecReg) {
+      // SCC will always be 0; use unconditional branch
----------------
Comment for the instruction pattern


================
Comment at: llvm/lib/Target/AMDGPU/SIInsertSkips.cpp:228-238
+  bool ReplaceSuccessor = MBB.succ_size() <= 1;
+  if (ReplaceSuccessor)
+    ReplaceSuccessor = tidySCCDef(MI);
+
+  MachineInstr *BranchMI = nullptr;
+  if (ReplaceSuccessor) {
+    // Branch is always taken
----------------
Why isn't BranchFolding handling this case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94748