[PATCH] D111223: [GlobalISel] Pass RegBankSelect to applyMapping

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/RegBankSelect.cpp:46
 #include <cstdint>
+#include <deque>
 #include <limits>
----------------
Don't need this since it's already done in the header.


================
Comment at: llvm/lib/CodeGen/GlobalISel/RegBankSelect.cpp:699
   ReversePostOrderTraversal<MachineFunction*> RPOT(&MF);
-  for (MachineBasicBlock *MBB : RPOT) {
-    // Set a sensible insertion point so that subsequent calls to
-    // MIRBuilder.
-    MIRBuilder.setMBB(*MBB);
-    for (MachineBasicBlock::iterator MII = MBB->begin(), End = MBB->end();
-         MII != End;) {
-      // MI might be invalidated by the assignment, so move the
-      // iterator before hand.
-      MachineInstr &MI = *MII++;
-
-      // Ignore target-specific post-isel instructions: they should use proper
-      // regclasses.
-      if (isTargetSpecificOpcode(MI.getOpcode()) && !MI.isPreISelOpcode())
-        continue;
-
-      // Ignore inline asm instructions: they should use physical
-      // registers/regclasses
-      if (MI.isInlineAsm())
-        continue;
-
-      // Ignore debug info.
-      if (MI.isDebugInstr())
-        continue;
-
-      // Ignore IMPLICIT_DEF which must have a regclass.
-      if (MI.isImplicitDef())
-        continue;
-
-      if (!assignInstr(MI)) {
-        reportGISelFailure(MF, *TPC, *MORE, "gisel-regbankselect",
-                           "unable to map instruction", MI);
-        return false;
-      }
-
-      // It's possible the mapping changed control flow, and moved the following
-      // instruction to a new block, so figure out the new parent.
-      if (MII != End) {
-        MachineBasicBlock *NextInstBB = MII->getParent();
-        if (NextInstBB != MBB) {
-          LLVM_DEBUG(dbgs() << "Instruction mapping changed control flow\n");
-          MBB = NextInstBB;
-          MIRBuilder.setMBB(*MBB);
-          End = MBB->end();
+  for (MachineBasicBlock *OrigMBB : RPOT) {
+    Worklist.push_back(OrigMBB);
----------------
Can you initialise the worklist from RPOT and then just have a single worklist loop, instead of two nested loops?


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp:774-776
+  RegBankSelectPass.addBBToWorkQueue(LoopBB);
+  RegBankSelectPass.addBBToWorkQueue(RemainderBB);
+  RegBankSelectPass.addBBToWorkQueue(RestoreExecBB);
----------------
Before this patch, how did these new BBs get regbankselected?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111223