[PATCH] D115747: [AMDGPU] Flush the vmcnt counter in loop preheader when necessary

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineBasicBlock.cpp:919
+const MachineBasicBlock *MachineBasicBlock::getSingleSuccessor() const {
+  const_succ_iterator SI = succ_begin(), E = succ_end();
+  if (SI == E)
----------------
MachineBasicBlock successors are stored in a vector so you can write this more simply e.g.: `return Successors.size() == 1 ? Successors[0] : nullptr;`


================
Comment at: llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp:448
+      AMDGPU::Waitcnt Wait,
+      std::function<MachineBasicBlock::instr_iterator()> GetIterator,
+      MachineBasicBlock &Block, WaitcntBrackets &ScoreBrackets,
----------------
Why can't you just pass in an iterator? As Nicolai said, they should not get invalidated.


================
Comment at: llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp:1252
+  MachineBasicBlock::instr_iterator It = GetIterator();
+  const DebugLoc &DL = (It == Block.instr_end()) ? Block.back().getDebugLoc()
+                                                 : It->getDebugLoc();
----------------
Maybe use MachineBasicBlock::findDebugLoc or similar?


================
Comment at: llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp:1559
 
+    bool FlushVmCnt = (Block.getFirstTerminator() == Inst) &&
+                      isPreheaderToFlush(Block, ScoreBrackets);
----------------
Don't need these parentheses.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115747