[LiveRegUnits] Add phys_regs_and_masks iterator range (NFC).

[fhahn]

Summary

This iterator range just includes physical registers and register masks,
which are interesting when dealing with register liveness.

[fhahn]

ping

[paquette]

llvm/lib/CodeGen/LivePhysRegs.cpp

47

In this patch, we don't have an equivalent check to O->isDebug(). Instead, all we have is a check for MOP.isDef().

Do we need a similar check in the new code?

[fhahn]

Filter out debug operands and add doxygen comment to function.

[fhahn]

llvm/lib/CodeGen/LivePhysRegs.cpp

47

Yep, I originally thought that Debug operands cannot also be physical registers, but given that all the existing places check for it I guess its safer to explicitly filter them out. I've update the patch ( and also added a comment to phys_regs_and_masks)

[merge_guards_bot]

Build result: pass - 60637 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt, CMakeCache.txt

[paquette]

LGTM!

[jinlin]

Hi Florian,

I have encountered a big compile time regression in the machine outlining caused by https://reviews.llvm.org/D70562.

After I backed out the change in D70562, the compile time for the three apps below was back.

I believe the reason is due to the introduction of function phys_regs_and_masks as below. Originally there is only one loop. With this change, it becomes two loops since it collects the legal operands first. If the filter does not have any effect, it essentially means the loop trip count is doubled.

for (ConstMIBundleOperands O(MI); O.isValid(); ++O) {

>

for (const MachineOperand &MOP : phys_regs_and_masks(MI)) {

When I back out the change D70562, I have also made minor changes in file llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp since phys_regs_and_masks is used in that file.

app1                 app2               app3

Build Time (LLVM 9.0) 4278.512s 2547.800s 5917.305s

Build Time (LLVM 10.0) 5625.795s 4254.092s 9474.335s

Build Time (LLVM 10.0 + backout D70562) 4757.243s (+15%) 2618.070s (+42%) 6996.272s (+26%)

[fhahn]

In D70562#2425971, @jinlin wrote:

Hi Florian,

I have encountered a big compile time regression in the machine outlining caused by https://reviews.llvm.org/D70562.

After I backed out the change in D70562, the compile time for the three apps below was back.

I believe the reason is due to the introduction of function phys_regs_and_masks as below. Originally there is only one loop. With this change, it becomes two loops since it collects the legal operands first. If the filter does not have any effect, it essentially means the loop trip count is doubled.

for (ConstMIBundleOperands O(MI); O.isValid(); ++O) {

>

for (const MachineOperand &MOP : phys_regs_and_masks(MI)) {

When I back out the change D70562, I have also made minor changes in file llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp since phys_regs_and_masks is used in that file.

app1                 app2               app3

Build Time (LLVM 9.0) 4278.512s 2547.800s 5917.305s

Build Time (LLVM 10.0) 5625.795s 4254.092s 9474.335s

Build Time (LLVM 10.0 + backout D70562) 4757.243s (+15%) 2618.070s (+42%) 6996.272s (+26%)

Let me take a look. Are the apps accessible somewhere? Or could be you provide a MIR/bitcode file to reproduce?

[jinlin]

Sorry I could not provide bitcode file since it is related to IP issue.

In D70562#2429184, @fhahn wrote:

Let me take a look. Are the apps accessible somewhere? Or could be you provide a MIR/bitcode file to reproduce?

llvm/include/llvm/CodeGen/LiveRegUnits.h

public:
   }
 
 private:
   /// Adds pristine registers. Pristine registers are callee saved registers
   /// that are unused in the function.
   void addPristines(const MachineFunction &MF);
 };
 
+inline iterator_range<filter_iterator<
+    ConstMIBundleOperands, std::function<bool(const MachineOperand &)>>>
+phys_regs_and_masks(const MachineInstr &MI) {
+  std::function<bool(const MachineOperand &)> Pred =
+      [](const MachineOperand &MOP) {
+        return MOP.isRegMask() ||
+               (MOP.isReg() && Register::isPhysicalRegister(MOP.getReg()));
+      };
+  return make_filter_range(const_mi_bundle_ops(MI), Pred);
+}
+
 } // end namespace llvm
 
 #endif // LLVM_CODEGEN_LIVEREGUNITS_H

llvm/lib/CodeGen/LivePhysRegs.cpp

 //===--- LivePhysRegs.cpp - Live Physical Register Set --------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the LivePhysRegs utility for tracking liveness of
 // physical registers across machine instructions in forward or backward order.
 // A more detailed description can be found in the corresponding header file.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/CodeGen/LivePhysRegs.h"
+#include "llvm/CodeGen/LiveRegUnits.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineInstrBundle.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace llvm;
if (MO.clobbersPhysReg(*LRI)) {
       LRI = LiveRegs.erase(LRI);
     } else
       ++LRI;
   }
 }
 
 /// Remove defined registers and regmask kills from the set.
 void LivePhysRegs::removeDefs(const MachineInstr &MI) {
-  for (ConstMIBundleOperands O(MI); O.isValid(); ++O) {
-    if (O->isReg()) {
-      if (!O->isDef() || O->isDebug())
-        continue;
-      Register Reg = O->getReg();
-      if (!Register::isPhysicalRegister(Reg))
-        continue;
-      removeReg(Reg);
-    } else if (O->isRegMask())
-      removeRegsInMask(*O);
+  for (const MachineOperand &MOP : phys_regs_and_masks(MI)) {
+    if (MOP.isRegMask()) {
+      removeRegsInMask(MOP);
+    }
+
+    if (MOP.isDef())
+      removeReg(MOP.getReg());
   }
 }
 
 /// Add uses to the set.
 void LivePhysRegs::addUses(const MachineInstr &MI) {
-  for (ConstMIBundleOperands O(MI); O.isValid(); ++O) {
-    if (!O->isReg() || !O->readsReg() || O->isDebug())
-      continue;
-    Register Reg = O->getReg();
-    if (!Register::isPhysicalRegister(Reg))
+  for (const MachineOperand &MOP : phys_regs_and_masks(MI)) {
+    if (!MOP.isReg() || !MOP.readsReg())
       continue;
-    addReg(Reg);
+    addReg(MOP.getReg());
   }
 }
 
 /// Simulates liveness when stepping backwards over an instruction(bundle):
 /// Remove Defs, add uses. This is the recommended way of calculating liveness.
 void LivePhysRegs::stepBackward(const MachineInstr &MI) {
   // Remove defined registers and regmask kills from the set.
   removeDefs(MI);

llvm/lib/CodeGen/LiveRegUnits.cpp

for (MCRegUnitRootIterator RootReg(U, TRI); RootReg.isValid(); ++RootReg) {
       if (MachineOperand::clobbersPhysReg(RegMask, *RootReg))
         Units.set(U);
     }
   }
 }
 
 void LiveRegUnits::stepBackward(const MachineInstr &MI) {
   // Remove defined registers and regmask kills from the set.
-  for (ConstMIBundleOperands O(MI); O.isValid(); ++O) {
-    if (O->isReg()) {
-      if (!O->isDef() || O->isDebug())
-        continue;
-      Register Reg = O->getReg();
-      if (!Register::isPhysicalRegister(Reg))
-        continue;
-      removeReg(Reg);
-    } else if (O->isRegMask())
-      removeRegsNotPreserved(O->getRegMask());
+  for (const MachineOperand &MOP : phys_regs_and_masks(MI)) {
+    if (MOP.isRegMask()) {
+      removeRegsNotPreserved(MOP.getRegMask());
+    }
+
+    if (MOP.isDef())
+      removeReg(MOP.getReg());
   }
 
   // Add uses to the set.
-  for (ConstMIBundleOperands O(MI); O.isValid(); ++O) {
-    if (!O->isReg() || !O->readsReg() || O->isDebug())
+  for (const MachineOperand &MOP : phys_regs_and_masks(MI)) {
+    if (!MOP.isReg() || !MOP.readsReg())
       continue;
-    Register Reg = O->getReg();
-    if (!Register::isPhysicalRegister(Reg))
-      continue;
-    addReg(Reg);
+    addReg(MOP.getReg());
   }
 }
 
 void LiveRegUnits::accumulate(const MachineInstr &MI) {
   // Add defs, uses and regmask clobbers to the set.
-  for (ConstMIBundleOperands O(MI); O.isValid(); ++O) {
-    if (O->isReg()) {
-      Register Reg = O->getReg();
-      if (!Register::isPhysicalRegister(Reg))
-        continue;
-      if (!O->isDef() && !O->readsReg())
-        continue;
-      addReg(Reg);
-    } else if (O->isRegMask())
-      addRegsInMask(O->getRegMask());
+  for (const MachineOperand &MOP : phys_regs_and_masks(MI)) {
+    if (MOP.isRegMask()) {
+      addRegsInMask(MOP.getRegMask());
+    }
+    if (!MOP.isDef() && !MOP.readsReg())
+    addReg(MOP.getReg());
   }
 }
 
 /// Add live-in registers of basic block \p MBB to \p LiveUnits.
 static void addBlockLiveIns(LiveRegUnits &LiveUnits,
                             const MachineBasicBlock &MBB) {
   for (const auto &LI : MBB.liveins())
     LiveUnits.addRegMasked(LI.PhysReg, LI.LaneMask);