[MachineOperand][Target] MachineOperand::isRenamable semantics changes

Summary:
Add a target option AllowRegisterRenaming that is used to opt in to
post-register-allocation renaming of registers.  This is set to 0 by
default, which causes the hasExtraSrcRegAllocReq/hasExtraDstRegAllocReq
fields of all opcodes to be set to 1, causing
MachineOperand::isRenamable to always return false.

Set the AllowRegisterRenaming flag to 1 for all in-tree targets that
have lit tests that were effected by enabling COPY forwarding in
MachineCopyPropagation (AArch64, AMDGPU, ARM, Hexagon, Mips, PowerPC,
RISCV, Sparc, SystemZ and X86).

Add some more comments describing the semantics of the
MachineOperand::isRenamable function and how it is set and maintained.

Change isRenamable to check the operand's opcode
hasExtraSrcRegAllocReq/hasExtraDstRegAllocReq bit directly instead of
relying on it being consistently reflected in the IsRenamable bit
setting.

Clear the IsRenamable bit when changing an operand's register value.

Remove target code that was clearing the IsRenamable bit when changing
registers/opcodes now that this is done conservatively by default.

Change setting of hasExtraSrcRegAllocReq in AMDGPU target to be done in
one place covering all opcodes that have constant pipe read limit
restrictions.

Reviewers: qcolombet, MatzeB

Subscribers: aemerson, arsenm, jyknight, mcrosier, sdardis, nhaehnle, javed.absar, tpr, arichardson, kristof.beyls, kbarton, fedor.sergeev, asb, rbar, johnrusso, simoncook, jordy.potman.lists, apazos, sabuasal, niosHD, escha, nemanjai, llvm-commits

Differential Revision: https://reviews.llvm.org/D43042

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@325931 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/CodeGen/MachineInstr.h

@@ -1126,8 +1126,7 @@ class MachineInstr
   /// Replace all occurrences of FromReg with ToReg:SubIdx,
   /// properly composing subreg indices where necessary.
   void substituteRegister(unsigned FromReg, unsigned ToReg, unsigned SubIdx,
-                          const TargetRegisterInfo &RegInfo,
-                          bool ClearIsRenamable = false);
+                          const TargetRegisterInfo &RegInfo);
 
   /// We have determined MI kills a register. Look for the
   /// operand that uses it and mark it as IsKill. If AddIfNotFound is true,

include/llvm/CodeGen/MachineOperand.h

@@ -105,11 +105,7 @@ class MachineOperand {
   /// This is only valid on register operands.
   unsigned IsDeadOrKill : 1;
 
-  /// IsRenamable - True if this register may be renamed, i.e. it does not
-  /// generate a value that is somehow read in a way that is not represented by
-  /// the Machine IR (e.g. to meet an ABI or ISA requirement).  This is only
-  /// valid on physical register operands.  Virtual registers are assumed to
-  /// always be renamable regardless of the value of this field.
+  /// See isRenamable().
   unsigned IsRenamable : 1;
 
   /// IsUndef - True if this register operand reads an "undef" value, i.e. the
@@ -384,6 +380,35 @@ class MachineOperand {
     return IsUndef;
   }
 
+  /// isRenamable - Returns true if this register may be renamed, i.e. it does
+  /// not generate a value that is somehow read in a way that is not represented
+  /// by the Machine IR (e.g. to meet an ABI or ISA requirement).  This is only
+  /// valid on physical register operands.  Virtual registers are assumed to
+  /// always be renamable regardless of the value of this field.
+  ///
+  /// Operands that are renamable can freely be changed to any other register
+  /// that is a member of the register class returned by
+  /// MI->getRegClassConstraint().
+  ///
+  /// isRenamable can return false for several different reasons:
+  ///
+  /// - ABI constraints (since liveness is not always precisely modeled).  We
+  ///   conservatively handle these cases by setting all physical register
+  ///   operands that didn’t start out as virtual regs to not be renamable.
+  ///   Also any physical register operands created after register allocation or
+  ///   whose register is changed after register allocation will not be
+  ///   renamable.  This state is tracked in the MachineOperand::IsRenamable
+  ///   bit.
+  ///
+  /// - Opcode/target constraints: for opcodes that have complex register class
+  ///   requirements (e.g. that depend on other operands/instructions), we set
+  ///   hasExtraSrcRegAllocReq/hasExtraDstRegAllocReq in the machine opcode
+  ///   description.  Operands belonging to instructions with opcodes that are
+  ///   marked hasExtraSrcRegAllocReq/hasExtraDstRegAllocReq return false from
+  ///   isRenamable().  Additionally, the AllowRegisterRenaming target property
+  ///   prevents any operands from being marked renamable for targets that don't
+  ///   have detailed opcode hasExtraSrcRegAllocReq/hasExtraDstRegAllocReq
+  ///   values.
   bool isRenamable() const;
 
   bool isInternalRead() const {
@@ -473,10 +498,6 @@ class MachineOperand {
 
   void setIsRenamable(bool Val = true);
 
-  /// Set IsRenamable to true if there are no extra register allocation
-  /// requirements placed on this operand by the parent instruction's opcode.
-  void setIsRenamableIfNoExtraRegAllocReq();
-
   void setIsInternalRead(bool Val = true) {
     assert(isReg() && "Wrong MachineOperand mutator");
     IsInternalRead = Val;

include/llvm/Target/Target.td

@@ -1362,6 +1362,12 @@ class Target {
 
   // AssemblyWriters - The AsmWriter instances available for this target.
   list<AsmWriter> AssemblyWriters = [DefaultAsmWriter];
+
+  // AllowRegisterRenaming - Controls whether this target allows
+  // post-register-allocation renaming of registers.  This is done by
+  // setting hasExtraDefRegAllocReq and hasExtraSrcRegAllocReq to 1
+  // for all opcodes if this flag is set to 0.
+  int AllowRegisterRenaming = 0;
 }
 
 //===----------------------------------------------------------------------===//

lib/CodeGen/MachineInstr.cpp

@@ -930,20 +930,16 @@ void MachineInstr::clearKillInfo() {
 
 void MachineInstr::substituteRegister(unsigned FromReg, unsigned ToReg,
                                       unsigned SubIdx,
-                                      const TargetRegisterInfo &RegInfo,
-                                      bool ClearIsRenamable) {
+                                      const TargetRegisterInfo &RegInfo) {
   if (TargetRegisterInfo::isPhysicalRegister(ToReg)) {
     if (SubIdx)
       ToReg = RegInfo.getSubReg(ToReg, SubIdx);
     for (MachineOperand &MO : operands()) {
       if (!MO.isReg() || MO.getReg() != FromReg)
         continue;
       MO.substPhysReg(ToReg, RegInfo);
-      if (ClearIsRenamable)
-        MO.setIsRenamable(false);
     }
   } else {
-    assert(!ClearIsRenamable && "IsRenamable invalid for virtual registers");
     for (MachineOperand &MO : operands()) {
       if (!MO.isReg() || MO.getReg() != FromReg)
         continue;

lib/CodeGen/MachineOperand.cpp

@@ -50,6 +50,9 @@ void MachineOperand::setReg(unsigned Reg) {
   if (getReg() == Reg)
     return; // No change.
 
+  // Clear the IsRenamable bit to keep it conservatively correct.
+  IsRenamable = false;
+
   // Otherwise, we have to change the register.  If this operand is embedded
   // into a machine function, we need to update the old and new register's
   // use/def lists.
@@ -110,30 +113,27 @@ bool MachineOperand::isRenamable() const {
   assert(isReg() && "Wrong MachineOperand accessor");
   assert(TargetRegisterInfo::isPhysicalRegister(getReg()) &&
          "isRenamable should only be checked on physical registers");
-  return IsRenamable;
+  if (!IsRenamable)
+    return false;
+
+  const MachineInstr *MI = getParent();
+  if (!MI)
+    return true;
+
+  if (isDef())
+    return !MI->hasExtraDefRegAllocReq(MachineInstr::IgnoreBundle);
+
+  assert(isUse() && "Reg is not def or use");
+  return !MI->hasExtraSrcRegAllocReq(MachineInstr::IgnoreBundle);
 }
 
 void MachineOperand::setIsRenamable(bool Val) {
   assert(isReg() && "Wrong MachineOperand accessor");
   assert(TargetRegisterInfo::isPhysicalRegister(getReg()) &&
          "setIsRenamable should only be called on physical registers");
-  if (const MachineInstr *MI = getParent())
-    if ((isDef() && MI->hasExtraDefRegAllocReq()) ||
-        (isUse() && MI->hasExtraSrcRegAllocReq()))
-      assert(!Val && "isRenamable should be false for "
-                     "hasExtraDefRegAllocReq/hasExtraSrcRegAllocReq opcodes");
   IsRenamable = Val;
 }
 
-void MachineOperand::setIsRenamableIfNoExtraRegAllocReq() {
-  if (const MachineInstr *MI = getParent())
-    if ((isDef() && MI->hasExtraDefRegAllocReq()) ||
-        (isUse() && MI->hasExtraSrcRegAllocReq()))
-      return;
-
-  setIsRenamable(true);
-}
-
 // If this operand is currently a register operand, and if this is in a
 // function, deregister the operand from the register's use/def list.
 void MachineOperand::removeRegFromUses() {

lib/CodeGen/MachineVerifier.cpp

@@ -1132,14 +1132,10 @@ MachineVerifier::visitMachineOperand(const MachineOperand *MO, unsigned MONum) {
         }
       }
       if (MO->isRenamable()) {
-        if ((MO->isDef() && MI->hasExtraDefRegAllocReq()) ||
-            (MO->isUse() && MI->hasExtraSrcRegAllocReq()))
-          report("Illegal isRenamable setting for opcode with extra regalloc "
-                 "requirements",
-                 MO, MONum);
-        if (MRI->isReserved(Reg))
+        if (MRI->isReserved(Reg)) {
           report("isRenamable set on reserved register", MO, MONum);
-        return;
+          return;
+        }
       }
     } else {
       // Virtual register.

lib/CodeGen/RegAllocFast.cpp

@@ -699,13 +699,13 @@ bool RegAllocFast::setPhysReg(MachineInstr &MI, unsigned OpNum,
   bool Dead = MO.isDead();
   if (!MO.getSubReg()) {
     MO.setReg(PhysReg);
-    MO.setIsRenamableIfNoExtraRegAllocReq();
+    MO.setIsRenamable(true);
     return MO.isKill() || Dead;
   }
 
   // Handle subregister index.
   MO.setReg(PhysReg ? TRI->getSubReg(PhysReg, MO.getSubReg()) : 0);
-  MO.setIsRenamableIfNoExtraRegAllocReq();
+  MO.setIsRenamable(true);
   MO.setSubReg(0);
 
   // A kill flag implies killing the full register. Add corresponding super

lib/CodeGen/VirtRegMap.cpp

@@ -530,7 +530,7 @@ void VirtRegRewriter::rewrite() {
         // Rewrite. Note we could have used MachineOperand::substPhysReg(), but
         // we need the inlining here.
         MO.setReg(PhysReg);
-        MO.setIsRenamableIfNoExtraRegAllocReq();
+        MO.setIsRenamable(true);
       }
 
       // Add any missing super-register kills after rewriting the whole

lib/Target/AArch64/AArch64.td

@@ -538,4 +538,5 @@ def AArch64 : Target {
   let InstructionSet = AArch64InstrInfo;
   let AssemblyParserVariants = [GenericAsmParserVariant, AppleAsmParserVariant];
   let AssemblyWriters = [GenericAsmWriter, AppleAsmWriter];
+  let AllowRegisterRenaming = 1;
 }

lib/Target/AMDGPU/AMDGPU.td

@@ -686,6 +686,7 @@ def AMDGPU : Target {
                                 SDWA9AsmParserVariant,
                                 DPPAsmParserVariant];
   let AssemblyWriters = [AMDGPUAsmWriter];
+  let AllowRegisterRenaming = 1;
 }
 
 // Dummy Instruction itineraries for pseudo instructions

lib/Target/AMDGPU/SIInsertSkips.cpp

@@ -278,11 +278,6 @@ void SIInsertSkips::kill(MachineInstr &MI) {
           .add(MI.getOperand(0))
           .addImm(0);  // omod
     }
-    // Clear isRenamable bit if new opcode requires it to be 0.
-    if (NewMI->hasExtraSrcRegAllocReq())
-      for (MachineOperand &NewMO : NewMI->uses())
-        if (NewMO.isReg() && NewMO.isUse())
-          NewMO.setIsRenamable(false);
     break;
   }
   case AMDGPU::SI_KILL_I1_TERMINATOR: {

lib/Target/AMDGPU/SIInstrFormats.td

@@ -186,6 +186,9 @@ class InstSI <dag outs, dag ins, string asm = "",
 
   let isAsmParserOnly = !if(!eq(DisableDecoder{0}, {0}), 0, 1);
   let AsmVariantName = AMDGPUAsmVariants.Default;
+
+  // Avoid changing source registers in a way that violates constant bus read limitations.
+  let hasExtraSrcRegAllocReq = !if(VOP1,1,!if(VOP2,1,!if(VOP3,1,!if(VOPC,1,!if(SDWA,1, !if(VALU,1,0))))));
 }
 
 class PseudoInstSI<dag outs, dag ins, list<dag> pattern = [], string asm = "">
@@ -203,8 +206,6 @@ class VPseudoInstSI<dag outs, dag ins, list<dag> pattern = [], string asm = "">
   : PseudoInstSI<outs, ins, pattern, asm> {
   let VALU = 1;
   let Uses = [EXEC];
-  // Avoid changing source registers in a way that violates constant bus read limitations.
-  let hasExtraSrcRegAllocReq = 1;
 }
 
 class CFPseudoInstSI<dag outs, dag ins, list<dag> pattern = [],

lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

@@ -246,7 +246,6 @@ bool SIOptimizeExecMasking::runOnMachineFunction(MachineFunction &MF) {
         DEBUG(dbgs() << "Fold exec copy: " << *PrepareExecInst);
 
         PrepareExecInst->getOperand(0).setReg(AMDGPU::EXEC);
-        PrepareExecInst->getOperand(0).setIsRenamable(false);
 
         DEBUG(dbgs() << "into: " << *PrepareExecInst << '\n');
 
@@ -353,8 +352,7 @@ bool SIOptimizeExecMasking::runOnMachineFunction(MachineFunction &MF) {
 
     for (MachineInstr *OtherInst : OtherUseInsts) {
       OtherInst->substituteRegister(CopyToExec, AMDGPU::EXEC,
-                                    AMDGPU::NoSubRegister, *TRI,
-                                    /*ClearIsRenamable=*/true);
+                                    AMDGPU::NoSubRegister, *TRI);
     }
   }
 

lib/Target/AMDGPU/VOPInstructions.td

@@ -81,8 +81,6 @@ class VOP3_Pseudo <string opName, VOPProfile P, list<dag> pattern = [],
   let UseNamedOperandTable = 1;
   let VOP3_OPSEL = isVop3OpSel;
   let IsPacked = P.IsPacked;
-  // Avoid changing source registers in a way that violates constant bus read limitations.
-  let hasExtraSrcRegAllocReq = 1;
 
   string Mnemonic = opName;
   string AsmOperands = !if(isVop3OpSel,

lib/Target/ARM/ARM.td

@@ -1043,4 +1043,5 @@ def ARM : Target {
   let AssemblyWriters = [ARMAsmWriter];
   let AssemblyParsers = [ARMAsmParser];
   let AssemblyParserVariants = [ARMAsmParserVariant];
+  let AllowRegisterRenaming = 1;
 }

lib/Target/ARM/ARMBaseInstrInfo.cpp

@@ -1378,7 +1378,6 @@ void ARMBaseInstrInfo::expandMEMCPY(MachineBasicBlock::iterator MI) const {
   MachineInstrBuilder LDM, STM;
   if (isThumb1 || !MI->getOperand(1).isDead()) {
     MachineOperand LDWb(MI->getOperand(1));
-    LDWb.setIsRenamable(false);
     LDM = BuildMI(*BB, MI, dl, TII->get(isThumb2 ? ARM::t2LDMIA_UPD
                                                  : isThumb1 ? ARM::tLDMIA_UPD
                                                             : ARM::LDMIA_UPD))
@@ -1389,7 +1388,6 @@ void ARMBaseInstrInfo::expandMEMCPY(MachineBasicBlock::iterator MI) const {
 
   if (isThumb1 || !MI->getOperand(0).isDead()) {
     MachineOperand STWb(MI->getOperand(0));
-    STWb.setIsRenamable(false);
     STM = BuildMI(*BB, MI, dl, TII->get(isThumb2 ? ARM::t2STMIA_UPD
                                                  : isThumb1 ? ARM::tSTMIA_UPD
                                                             : ARM::STMIA_UPD))
@@ -1399,11 +1397,9 @@ void ARMBaseInstrInfo::expandMEMCPY(MachineBasicBlock::iterator MI) const {
   }
 
   MachineOperand LDBase(MI->getOperand(3));
-  LDBase.setIsRenamable(false);
   LDM.add(LDBase).add(predOps(ARMCC::AL));
 
   MachineOperand STBase(MI->getOperand(2));
-  STBase.setIsRenamable(false);
   STM.add(STBase).add(predOps(ARMCC::AL));
 
   // Sort the scratch registers into ascending order.

lib/Target/ARM/ARMExpandPseudoInsts.cpp

@@ -608,7 +608,6 @@ void ARMExpandPseudo::ExpandVTBL(MachineBasicBlock::iterator &MBBI,
   MIB.add(MI.getOperand(OpIdx++));
   if (IsExt) {
     MachineOperand VdSrc(MI.getOperand(OpIdx++));
-    VdSrc.setIsRenamable(false);
     MIB.add(VdSrc);
   }
 
@@ -620,7 +619,6 @@ void ARMExpandPseudo::ExpandVTBL(MachineBasicBlock::iterator &MBBI,
 
   // Copy the other source register operand.
   MachineOperand VmSrc(MI.getOperand(OpIdx++));
-  VmSrc.setIsRenamable(false);
   MIB.add(VmSrc);
 
   // Copy the predicate operands.
@@ -1470,7 +1468,6 @@ bool ARMExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
 
       // Copy the destination register.
       MachineOperand Dst(MI.getOperand(OpIdx++));
-      Dst.setIsRenamable(false);
       MIB.add(Dst);
 
       // Copy the predicate operands.

lib/Target/ARM/ARMISelLowering.cpp

@@ -9196,8 +9196,6 @@ ARMTargetLowering::EmitInstrWithCustomInserter(MachineInstr &MI,
   // Thumb1 post-indexed loads are really just single-register LDMs.
   case ARM::tLDR_postidx: {
     MachineOperand Def(MI.getOperand(1));
-    if (TargetRegisterInfo::isPhysicalRegister(Def.getReg()))
-      Def.setIsRenamable(false);
     BuildMI(*BB, MI, dl, TII->get(ARM::tLDMIA_UPD))
         .add(Def)  // Rn_wb
         .add(MI.getOperand(2))  // Rn

lib/Target/Hexagon/Hexagon.td

@@ -359,4 +359,5 @@ def Hexagon : Target {
   let AssemblyParsers = [HexagonAsmParser];
   let AssemblyParserVariants = [HexagonAsmParserVariant];
   let AssemblyWriters = [HexagonAsmWriter];
+  let AllowRegisterRenaming = 1;
 }

lib/Target/Hexagon/RDFCopy.cpp

@@ -101,7 +101,6 @@ NodeId CopyPropagation::getLocalReachingDef(RegisterRef RefRR,
 
 bool CopyPropagation::run() {
   scanBlock(&DFG.getMF().front());
-  MachineRegisterInfo &MRI = DFG.getMF().getRegInfo();
 
   if (trace()) {
     dbgs() << "Copies:\n";
@@ -181,8 +180,6 @@ bool CopyPropagation::run() {
         unsigned NewReg = MinPhysReg(SR);
         Op.setReg(NewReg);
         Op.setSubReg(0);
-        if (MRI.isReserved(NewReg))
-          Op.setIsRenamable(false);
         DFG.unlinkUse(UA, false);
         if (AtCopy != 0) {
           UA.Addr->linkToDef(UA.Id, DFG.addr<DefNode*>(AtCopy));

lib/Target/Mips/Mips.td

@@ -242,4 +242,5 @@ def Mips : Target {
   let InstructionSet = MipsInstrInfo;
   let AssemblyParsers = [MipsAsmParser];
   let AssemblyParserVariants = [MipsAsmParserVariant];
+  let AllowRegisterRenaming = 1;
 }

lib/Target/PowerPC/PPC.td

@@ -465,4 +465,5 @@ def PPC : Target {
 
   let AssemblyParsers = [PPCAsmParser];
   let AssemblyParserVariants = [PPCAsmParserVariant];
+  let AllowRegisterRenaming = 1;
 }

lib/Target/RISCV/RISCV.td

@@ -92,4 +92,5 @@ def RISCV : Target {
   let InstructionSet = RISCVInstrInfo;
   let AssemblyParsers = [RISCVAsmParser];
   let AssemblyWriters = [RISCVAsmWriter];
+  let AllowRegisterRenaming = 1;
 }

lib/Target/Sparc/Sparc.td

@@ -176,4 +176,5 @@ def Sparc : Target {
   let InstructionSet = SparcInstrInfo;
   let AssemblyParsers  = [SparcAsmParser];
   let AssemblyWriters = [SparcAsmWriter];
+  let AllowRegisterRenaming = 1;
 }

lib/Target/SystemZ/SystemZ.td

@@ -75,4 +75,5 @@ def SystemZAsmParser : AsmParser {
 def SystemZ : Target {
   let InstructionSet = SystemZInstrInfo;
   let AssemblyParsers = [SystemZAsmParser];
+  let AllowRegisterRenaming = 1;
 }

lib/Target/X86/X86.td

@@ -1130,4 +1130,5 @@ def X86 : Target {
   let InstructionSet = X86InstrInfo;
   let AssemblyParserVariants = [ATTAsmParserVariant, IntelAsmParserVariant];
   let AssemblyWriters = [ATTAsmWriter, IntelAsmWriter];
+  let AllowRegisterRenaming = 1;
 }