[PATCH] D75504: [AMDGPU] moving vcc branch optimization into peephole

[Christudasan Devadasan via Phabricator via llvm-commits]

cdevadas marked 2 inline comments as done.
cdevadas added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp:1017
   addPass(&SIRemoveShortExecBranchesID);
+  addPass(&PeepholeOptimizerID);
   addPass(&SIInsertSkipsPassID);
----------------
arsenm wrote:
> cdevadas wrote:
> > arsenm wrote:
> > > Should not need an extra run of this
> > The peephole is invoked earlier during SSAOptimization. It is required here to optimize the pattern introduced later. The lit test multilevel-break.ll has a similar opportunity in function multi_if_break_loop. 
> Where is the pattern introduced? Does this ever trigger in the initial run?
The pattern got introduced with Basic Block Placement (See below, BB.2 & bb.5 are combined into BB.2)
The extra run is required to optimize it.

IR Dump before BB Placement:
multi_if_break_loop:
bb.2:
  successors: %bb.5
  liveins: $vgpr0, $vgpr1, $sgpr0_sgpr1_sgpr2_sgpr3:0x0000000C, $sgpr0_sgpr1, $sgpr4_sgpr5
  renamable $sgpr8_sgpr9 = S_MOV_B64 0
  renamable $sgpr6_sgpr7 = S_MOV_B64 -1
  renamable $sgpr10_sgpr11 = S_MOV_B64 -1
  S_BRANCH %bb.5
bb.3:
 // Insns
bb.5:
  successors: %bb.6, %bb.8
  renamable $vcc = S_AND_B64 $exec, killed renamable $sgpr10_sgpr11, implicit-def dead $scc
  S_CBRANCH_VCCZ %bb.8, implicit $vcc
---
IR Dump after BB Placement:
multi_if_break_loop:

bb.2:
  successors: %bb.6, %bb.8
  liveins: $vgpr0, $vgpr1, $sgpr0_sgpr1_sgpr2_sgpr3:0x0000000C, $sgpr0_sgpr1, $sgpr4_sgpr5
  renamable $sgpr8_sgpr9 = S_MOV_B64 0
  renamable $sgpr6_sgpr7 = S_MOV_B64 -1
  renamable $sgpr10_sgpr11 = S_MOV_B64 -1
  renamable $vcc = S_AND_B64 $exec, killed renamable $sgpr10_sgpr11, implicit-def dead $scc
  S_CBRANCH_VCCZ %bb.8, implicit $vcc


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:2182
+
+    unsigned SReg = AMDGPU::NoRegister;
+    if (Op2.isReg()) {
----------------
arsenm wrote:
> cdevadas wrote:
> > arsenm wrote:
> > > Register, and initialization isn't needed
> > Are you saying the initialization is not required? 
> > SReg is not defined in all control-flows later.
> If you use Register instead of unsigned, it default initializes to NoRegister/0
Sure, will do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75504