GlobalISel: Make MachineCSE runnable in the middle of the GlobalISel

bogner · bogner · commit a9346e050f30 · 2018-01-18T02:06:56.000Z
Right now, it is not possible to run MachineCSE in the middle of the
GlobalISel pipeline. Being able to run generic optimizations between the
core passes of GlobalISel was one of the goals of the new ISel framework.
This is the first attempt to do it.

The problem is that MachineCSE pass assumes all register operands have a
register class, which, in GlobalISel context, won't be true until after the
InstructionSelect pass. The reason for this behaviour is that before
replacing one virtual register with another, MachineCSE pass (and most of
the other optimization machine passes) must check if the virtual registers'
constraints have a (sufficiently large) intersection, and constrain the
resulting register appropriately if such intersection exists.

GlobalISel extends the representation of such constraints from just a
register class to a triple (low-level type, register bank, register
class).

This commit adds MachineRegisterInfo::constrainRegAttrs method that extends
MachineRegisterInfo::constrainRegClass to such a triple.

The idea is that going forward we should use:

- RegisterBankInfo::constrainGenericRegister within GlobalISel's
  InstructionSelect pass
- MachineRegisterInfo::constrainRegClass within SelectionDAG ISel
- MachineRegisterInfo::constrainRegAttrs everywhere else regardless
  the target and instruction selector it uses.

Patch by Roman Tereshin. Thanks!

llvm-svn: 322805
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/RegisterBankInfo.h b/llvm/include/llvm/CodeGen/GlobalISel/RegisterBankInfo.h
@@ -622,6 +622,8 @@ class RegisterBankInfo {
   /// \pre \p Reg is a virtual register that either has a bank or a class.
   /// \returns The constrained register class, or nullptr if there is none.
   /// \note This is a generic variant of MachineRegisterInfo::constrainRegClass
+  /// \note Use MachineRegisterInfo::constrainRegAttrs instead for any non-isel
+  /// purpose, including non-select passes of GlobalISel
   static const TargetRegisterClass *
   constrainGenericRegister(unsigned Reg, const TargetRegisterClass &RC,
                            MachineRegisterInfo &MRI);
diff --git a/llvm/include/llvm/CodeGen/MachineRegisterInfo.h b/llvm/include/llvm/CodeGen/MachineRegisterInfo.h
@@ -548,12 +548,16 @@ class MachineRegisterInfo {
   /// except that it also changes any definitions of the register as well.
   ///
   /// Note that it is usually necessary to first constrain ToReg's register
-  /// class to match the FromReg constraints using:
+  /// class and register bank to match the FromReg constraints using one of the
+  /// methods:
   ///
   ///   constrainRegClass(ToReg, getRegClass(FromReg))
+  ///   constrainRegAttrs(ToReg, FromReg)
+  ///   RegisterBankInfo::constrainGenericRegister(ToReg,
+  ///       *MRI.getRegClass(FromReg), MRI)
   ///
-  /// That function will return NULL if the virtual registers have incompatible
-  /// constraints.
+  /// These functions will return a falsy result if the virtual registers have
+  /// incompatible constraints.
   ///
   /// Note that if ToReg is a physical register the function will replace and
   /// apply sub registers to ToReg in order to obtain a final/proper physical
@@ -653,10 +657,30 @@ class MachineRegisterInfo {
   /// new register class, or NULL if no such class exists.
   /// This should only be used when the constraint is known to be trivial, like
   /// GR32 -> GR32_NOSP. Beware of increasing register pressure.
+  ///
+  /// \note Assumes that the register has a register class assigned.
+  /// Use RegisterBankInfo::constrainGenericRegister in GlobalISel's
+  /// InstructionSelect pass and constrainRegAttrs in every other pass,
+  /// including non-select passes of GlobalISel, instead.
   const TargetRegisterClass *constrainRegClass(unsigned Reg,
                                                const TargetRegisterClass *RC,
                                                unsigned MinNumRegs = 0);
 
+  /// Constrain the register class or the register bank of the virtual register
+  /// \p Reg to be a common subclass and a common bank of both registers
+  /// provided respectively. Do nothing if any of the attributes (classes,
+  /// banks, or low-level types) of the registers are deemed incompatible, or if
+  /// the resulting register will have a class smaller than before and of size
+  /// less than \p MinNumRegs. Return true if such register attributes exist,
+  /// false otherwise.
+  ///
+  /// \note Assumes that each register has either a low-level type or a class
+  /// assigned, but not both. Use this method instead of constrainRegClass and
+  /// RegisterBankInfo::constrainGenericRegister everywhere but SelectionDAG
+  /// ISel / FastISel and GlobalISel's InstructionSelect pass respectively.
+  bool constrainRegAttrs(unsigned Reg, unsigned ConstrainingReg,
+                         unsigned MinNumRegs = 0);
+
   /// recomputeRegClass - Try to find a legal super-class of Reg's register
   /// class that still satisfies the constraints from the instructions using
   /// Reg.  Returns true if Reg was upgraded.
diff --git a/llvm/lib/CodeGen/MachineCSE.cpp b/llvm/lib/CodeGen/MachineCSE.cpp
@@ -176,8 +176,7 @@ bool MachineCSE::PerformTrivialCopyPropagation(MachineInstr *MI,
     // class given a super-reg class and subreg index.
     if (DefMI->getOperand(1).getSubReg())
       continue;
-    const TargetRegisterClass *RC = MRI->getRegClass(Reg);
-    if (!MRI->constrainRegClass(SrcReg, RC))
+    if (!MRI->constrainRegAttrs(SrcReg, Reg))
       continue;
     DEBUG(dbgs() << "Coalescing: " << *DefMI);
     DEBUG(dbgs() << "***     to: " << *MI);
@@ -588,11 +587,11 @@ bool MachineCSE::ProcessBlock(MachineBasicBlock *MBB) {
         break;
       }
 
-      // Don't perform CSE if the result of the old instruction cannot exist
-      // within the register class of the new instruction.
-      const TargetRegisterClass *OldRC = MRI->getRegClass(OldReg);
-      if (!MRI->constrainRegClass(NewReg, OldRC)) {
-        DEBUG(dbgs() << "*** Not the same register class, avoid CSE!\n");
+      // Don't perform CSE if the result of the new instruction cannot exist
+      // within the constraints (register class, bank, or low-level type) of
+      // the old instruction.
+      if (!MRI->constrainRegAttrs(NewReg, OldReg)) {
+        DEBUG(dbgs() << "*** Not the same register constraints, avoid CSE!\n");
         DoCSE = false;
         break;
       }
diff --git a/llvm/lib/CodeGen/MachineRegisterInfo.cpp b/llvm/lib/CodeGen/MachineRegisterInfo.cpp
@@ -65,23 +65,66 @@ void MachineRegisterInfo::setRegBank(unsigned Reg,
   VRegInfo[Reg].first = &RegBank;
 }
 
-const TargetRegisterClass *
-MachineRegisterInfo::constrainRegClass(unsigned Reg,
-                                       const TargetRegisterClass *RC,
-                                       unsigned MinNumRegs) {
-  const TargetRegisterClass *OldRC = getRegClass(Reg);
+static const TargetRegisterClass *
+constrainRegClass(MachineRegisterInfo &MRI, unsigned Reg,
+                  const TargetRegisterClass *OldRC,
+                  const TargetRegisterClass *RC, unsigned MinNumRegs) {
   if (OldRC == RC)
     return RC;
   const TargetRegisterClass *NewRC =
-    getTargetRegisterInfo()->getCommonSubClass(OldRC, RC);
+      MRI.getTargetRegisterInfo()->getCommonSubClass(OldRC, RC);
   if (!NewRC || NewRC == OldRC)
     return NewRC;
   if (NewRC->getNumRegs() < MinNumRegs)
     return nullptr;
-  setRegClass(Reg, NewRC);
+  MRI.setRegClass(Reg, NewRC);
   return NewRC;
 }
 
+const TargetRegisterClass *
+MachineRegisterInfo::constrainRegClass(unsigned Reg,
+                                       const TargetRegisterClass *RC,
+                                       unsigned MinNumRegs) {
+  return ::constrainRegClass(*this, Reg, getRegClass(Reg), RC, MinNumRegs);
+}
+
+bool
+MachineRegisterInfo::constrainRegAttrs(unsigned Reg,
+                                       unsigned ConstrainingReg,
+                                       unsigned MinNumRegs) {
+  auto const *OldRC = getRegClassOrNull(Reg);
+  auto const *RC = getRegClassOrNull(ConstrainingReg);
+  // A virtual register at any point must have either a low-level type
+  // or a class assigned, but not both. The only exception is the internals of
+  // GlobalISel's instruction selection pass, which is allowed to temporarily
+  // introduce registers with types and classes both.
+  assert((OldRC || getType(Reg).isValid()) && "Reg has neither class nor type");
+  assert((!OldRC || !getType(Reg).isValid()) && "Reg has class and type both");
+  assert((RC || getType(ConstrainingReg).isValid()) &&
+         "ConstrainingReg has neither class nor type");
+  assert((!RC || !getType(ConstrainingReg).isValid()) &&
+         "ConstrainingReg has class and type both");
+  if (OldRC && RC)
+    return ::constrainRegClass(*this, Reg, OldRC, RC, MinNumRegs);
+  // If one of the virtual registers is generic (used in generic machine
+  // instructions, has a low-level type, doesn't have a class), and the other is
+  // concrete (used in target specific instructions, doesn't have a low-level
+  // type, has a class), we can not unify them.
+  if (OldRC || RC)
+    return false;
+  // At this point, both registers are guaranteed to have a valid low-level
+  // type, and they must agree.
+  if (getType(Reg) != getType(ConstrainingReg))
+    return false;
+  auto const *OldRB = getRegBankOrNull(Reg);
+  auto const *RB = getRegBankOrNull(ConstrainingReg);
+  if (OldRB)
+    return !RB || RB == OldRB;
+  if (RB)
+    setRegBank(Reg, *RB);
+  return true;
+}
+
 bool
 MachineRegisterInfo::recomputeRegClass(unsigned Reg) {
   const TargetInstrInfo *TII = MF->getSubtarget().getInstrInfo();
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/machine-cse-mid-pipeline.mir b/llvm/test/CodeGen/AArch64/GlobalISel/machine-cse-mid-pipeline.mir
@@ -0,0 +1,181 @@
+# RUN: llc -run-pass machine-cse -global-isel -verify-machineinstrs -mtriple aarch64-apple-ios %s -o - | FileCheck %s
+---
+name:            irtranslated
+legalized:       false
+regBankSelected: false
+selected:        false
+body:             |
+  ; CHECK-LABEL: name: irtranslated
+  ; CHECK:      %[[ONE:[0-9]+]]:_(s32) = G_CONSTANT i32 1
+  ; CHECK-NEXT: %[[TWO:[0-9]+]]:_(s32) = G_ADD %[[ONE]], %[[ONE]]
+  ; CHECK-NEXT: %[[SUM:[0-9]+]]:_(s32) = G_ADD %[[TWO]], %[[TWO]]
+  ; CHECK-NEXT: %[[RET:[wx][0-9]+]] = COPY %[[SUM]](s32)
+  ; CHECK-NEXT: RET_ReallyLR implicit %[[RET]]
+  bb.0:
+    %0:_(s32) = G_CONSTANT i32 1
+    %1:_(s32) = G_ADD %0, %0
+    %2:_(s32) = G_ADD %0, %0
+    %3:_(s32) = G_ADD %1, %2
+    %w0 = COPY %3(s32)
+    RET_ReallyLR implicit %w0
+...
+---
+name:            regbankselected
+legalized:       true
+regBankSelected: true
+selected:        false
+body:             |
+  ; CHECK-LABEL: name: regbankselected
+  ; CHECK:      %[[ONE:[0-9]+]]:gpr(s32) = G_CONSTANT i32 1
+  ; CHECK-NEXT: %[[TWO:[0-9]+]]:gpr(s32) = G_ADD %[[ONE]], %[[ONE]]
+  ; CHECK-NEXT: %[[SUM:[0-9]+]]:gpr(s32) = G_ADD %[[TWO]], %[[TWO]]
+  ; CHECK-NEXT: %[[RET:[wx][0-9]+]] = COPY %[[SUM]](s32)
+  ; CHECK-NEXT: RET_ReallyLR implicit %[[RET]]
+  bb.0:
+    %0:gpr(s32) = G_CONSTANT i32 1
+    %1:gpr(s32) = G_ADD %0, %0
+    %2:gpr(s32) = G_ADD %0, %0
+    %3:gpr(s32) = G_ADD %1, %2
+    %w0 = COPY %3(s32)
+    RET_ReallyLR implicit %w0
+...
+---
+name:            legalized
+legalized:       true
+regBankSelected: false
+selected:        false
+body:             |
+  ; CHECK-LABEL: name: legalized
+  ; CHECK:      %[[ONE:[0-9]+]]:_(s32) = G_CONSTANT i32 1
+  ; CHECK-NEXT: %[[TWO:[0-9]+]]:gpr(s32) = G_ADD %[[ONE]], %[[ONE]]
+  ; CHECK-NEXT: %[[SUM:[0-9]+]]:_(s32) = G_ADD %[[TWO]], %[[TWO]]
+  ; CHECK-NEXT: %[[RET:[wx][0-9]+]] = COPY %[[SUM]](s32)
+  ; CHECK-NEXT: RET_ReallyLR implicit %[[RET]]
+  bb.0:
+    %0:_(s32) = G_CONSTANT i32 1
+    %1:_(s32) = G_ADD %0, %0
+    %2:gpr(s32) = G_ADD %0, %0
+    %3:_(s32) = G_ADD %1, %2
+    %w0 = COPY %3(s32)
+    RET_ReallyLR implicit %w0
+...
+---
+name:            legalized_sym
+legalized:       true
+regBankSelected: false
+selected:        false
+body:             |
+  ; CHECK-LABEL: name: legalized_sym
+  ; CHECK:      %[[ONE:[0-9]+]]:_(s32) = G_CONSTANT i32 1
+  ; CHECK-NEXT: %[[TWO:[0-9]+]]:gpr(s32) = G_ADD %[[ONE]], %[[ONE]]
+  ; CHECK-NEXT: %[[SUM:[0-9]+]]:_(s32) = G_ADD %[[TWO]], %[[TWO]]
+  ; CHECK-NEXT: %[[RET:[wx][0-9]+]] = COPY %[[SUM]](s32)
+  ; CHECK-NEXT: RET_ReallyLR implicit %[[RET]]
+  bb.0:
+    %0:_(s32) = G_CONSTANT i32 1
+    %1:gpr(s32) = G_ADD %0, %0
+    %2:_(s32) = G_ADD %0, %0
+    %3:_(s32) = G_ADD %1, %2
+    %w0 = COPY %3(s32)
+    RET_ReallyLR implicit %w0
+...
+---
+name:            int_extensions
+alignment:       2
+legalized:       false
+regBankSelected: false
+selected:        false
+body:             |
+  ; CHECK-LABEL: name: int_extensions
+  ; CHECK:      %[[ONE:[0-9]+]]:_(s8) = G_CONSTANT i8 1
+  ; CHECK-NEXT: %[[S16:[0-9]+]]:_(s16) = G_SEXT %[[ONE]](s8)
+  ; CHECK-NEXT: %[[S32:[0-9]+]]:_(s32) = G_SEXT %[[ONE]](s8)
+  ; CHECK-NEXT: %[[S16_Z64:[0-9]+]]:_(s64) = G_ZEXT %[[S16]](s16)
+  ; CHECK-NEXT: %[[S32_Z64:[0-9]+]]:_(s64) = G_ZEXT %[[S32]](s32)
+  ; CHECK-NEXT: %[[SUM:[0-9]+]]:_(s64) = G_ADD %[[S16_Z64]], %[[S32_Z64]]
+  ; CHECK-NEXT: %[[RET:[wx][0-9]+]] = COPY %[[SUM]](s64)
+  ; CHECK-NEXT: RET_ReallyLR implicit %[[RET]]
+  bb.0.entry:
+    %0:_(s8) = G_CONSTANT i8 1
+    %1:_(s16) = G_SEXT %0(s8)
+    %2:_(s32) = G_SEXT %0(s8)
+    %3:_(s64) = G_ZEXT %1(s16)
+    %4:_(s64) = G_ZEXT %2(s32)
+    %5:_(s64) = G_ADD %3, %4
+    %x0 = COPY %5(s64)
+    RET_ReallyLR implicit %x0
+...
+---
+name:            generic
+legalized:       true
+regBankSelected: false
+selected:        false
+body:             |
+  ; CHECK-LABEL: name: generic
+  ; CHECK:      %[[SG:[0-9]+]]:_(s32) = G_ADD %{{[0-9]+}}, %{{[0-9]+}}
+  ; CHECK-NEXT: %{{[0-9]+}}:_(s32) = G_ADD %[[SG]], %[[SG]]
+  bb.0:
+    %0:_(s32) = COPY %w0
+    %1:_(s32) = COPY %w1
+    %2:_(s32) = G_ADD %0, %1
+    %3:_(s32) = COPY %2(s32)
+    %4:_(s32) = G_ADD %3, %3
+    %w0 = COPY %4(s32)
+    RET_ReallyLR implicit %w0
+...
+---
+name:            generic_to_concrete_copy
+legalized:       true
+regBankSelected: false
+selected:        false
+body:             |
+  ; CHECK-LABEL: name: generic_to_concrete_copy
+  ; CHECK:      %[[S1:[0-9]+]]:_(s32) = G_ADD %{{[0-9]+}}, %{{[0-9]+}}
+  ; CHECK-NEXT: %[[S2:[0-9]+]]:gpr32 = COPY %[[S1]](s32)
+  ; CHECK-NEXT: %{{[0-9]+}}:gpr32 = ADDWrr %[[S2]], %[[S2]]
+  bb.0:
+    %0:_(s32) = COPY %w0
+    %1:_(s32) = COPY %w1
+    %2:_(s32) = G_ADD %0, %1
+    %3:gpr32 = COPY %2(s32)
+    %4:gpr32 = ADDWrr %3, %3
+    %w0 = COPY %4
+    RET_ReallyLR implicit %w0
+...
+---
+name:            concrete_to_generic_copy
+legalized:       true
+regBankSelected: false
+selected:        false
+body:             |
+  ; CHECK-LABEL: name: concrete_to_generic_copy
+  ; CHECK:      %[[S1:[0-9]+]]:gpr32 = ADDWrr %{{[0-9]+}}, %{{[0-9]+}}
+  ; CHECK-NEXT: %[[S2:[0-9]+]]:_(s32) = COPY %[[S1]]
+  ; CHECK-NEXT: %{{[0-9]+}}:_(s32) = G_ADD %[[S2]], %[[S2]]
+  bb.0:
+    %0:gpr32 = COPY %w0
+    %1:gpr32 = COPY %w1
+    %2:gpr32 = ADDWrr %0, %1
+    %3:_(s32) = COPY %2
+    %4:_(s32) = G_ADD %3, %3
+    %w0 = COPY %4(s32)
+    RET_ReallyLR implicit %w0
+...
+---
+name:            concrete
+legalized:       true
+regBankSelected: false
+selected:        false
+body:             |
+  ; CHECK-LABEL: name: concrete
+  ; CHECK:      %[[SC:[0-9]+]]:gpr32 = ADDWrr %{{[0-9]+}}, %{{[0-9]+}}
+  ; CHECK-NEXT: %{{[0-9]+}}:gpr32 = ADDWrr %[[SC]], %[[SC]]
+  bb.0:
+    %0:gpr32 = COPY %w0
+    %1:gpr32 = COPY %w1
+    %2:gpr32 = ADDWrr %0, %1
+    %3:gpr32 = COPY %2
+    %4:gpr32 = ADDWrr %3, %3
+    %w0 = COPY %4
+    RET_ReallyLR implicit %w0
+...
