[PATCH] D109052: [AMDGPU][GlobalISel] Fix waterfall loops

Wed Oct 6 06:29:52 PDT 2021

foad added inline comments.

================
Comment at: llvm/lib/CodeGen/GlobalISel/Localizer.cpp:172

-    MachineBasicBlock::iterator II(MI);
+    MachineBasicBlock::instr_iterator II(MI);
     ++II;
----------------
Why is this change needed? Normally a BUNDLE instruction has use operands representing all the uses of the instructions inside the bundle, so I think this code should just work without any changes? See comment in lowerCall...

================
Comment at: llvm/lib/CodeGen/MachineVerifier.cpp:2273
+    if (!regsLive.count(Reg) &&
+        !RegSet(regsDefined.begin(), regsDefined.end()).count(Reg)) {
       if (Register::isPhysicalRegister(Reg)) {
----------------
Why is this change needed?

Anyway I think you can write `!is_contained(regsDefined, Reg)`.

================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUCallLowering.cpp:1426
+    auto &MBB = MIRBuilder.getMBB();
+    MIBundleBuilder Bundler(MBB, CallStart.getValue(),
+                            MIRBuilder.getInsertPt());
----------------
Use finalizeBundle instead of MIBundleBuilder, to add all the required operands to the BUNDLE instruction?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109052