TargetLowering: Add finalizeLowering() function; NFC

[MatzeB]

Summary

Adds a new method finalizeLowering to TargetLoweringBase. This is in preparation for an upcoming commit.

This function is meant for target specific adjustments to MachineFrameInfo or register reservations.

Move the freezeRegisters() and the hasCopyImplyingStackAdjustment() handling into the new function to prove the concept. As an added bonus GlobalISel no longer missed the hasCopyImplyingStackAdjustment() handling with this.

[MatzeB]

Slight change so this is actually a NFC.

[arsenm]

lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp

595 ↗

(On Diff #97024)

Can we move this to after ExpandISelPseudos at least? It's a problem that reserved registers are frozen this early

[MatzeB]

lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp

595 ↗

(On Diff #97024)

I think that should work, we certainly could try. Maybe rename ExpandISelPseudos to FinalizeISel then?

However I'd like to do that in a separate patch then, to avoid complications when backporting this to a production branches.

[arsenm]

lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp

595 ↗

(On Diff #97024)

Last time I tried to do that a few PPC tests broke, although that might have been fixed since then

[MatzeB]

lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp

595 ↗

(On Diff #97024)

Just tried it https://reviews.llvm.org/D32628 and it seems to work (at least for PowerPC). So if you need this feel free to grab it and finish it up (more testing, changing tests to adjust for the changed passname, ask GlobalISel people whether is is fine to freeze registers later).

[rnk]

lgtm

[rnk]

Oops, I went straight to the diff and missed Matt's comment.

[arsenm]

LGTM

[MatzeB]

In D32621#741035, @rnk wrote:

Oops, I went straight to the diff and missed Matt's comment.

Does that mean you want to see Matts comment addressed (I think we both agreed to do that in a separate patch), or just need more time for review?

[rnk]

In D32621#741133, @MatzeB wrote:

In D32621#741035, @rnk wrote:

Oops, I went straight to the diff and missed Matt's comment.

Does that mean you want to see Matts comment addressed (I think we both agreed to do that in a separate patch), or just need more time for review?

No, I looked at the other patch and that seems fine. Looks good!

llvm/trunk/include/llvm/Target/TargetLowering.h

void setLibcallCallingConv(RTLIB::Libcall Call, CallingConv::ID CC) {
     LibcallCallingConvs[Call] = CC;
   }
 
   /// Get the CallingConv that should be used for the specified libcall.
   CallingConv::ID getLibcallCallingConv(RTLIB::Libcall Call) const {
     return LibcallCallingConvs[Call];
   }
 
+  /// Execute target specific actions to finalize target lowering.
+  /// This is used to set extra flags in MachineFrameInformation and freezing
+  /// the set of reserved registers.
+  /// The default implementation just freezes the set of reserved registers.
+  virtual void finalizeLowering(MachineFunction &MF) const;
+
 private:
   const TargetMachine &TM;
 
   /// Tells the code generator that the target has multiple (allocatable)
   /// condition registers that can be used to store the results of comparisons
   /// for use by selects and conditional branches. With multiple condition
   /// registers, the code generator will not aggressively sink comparisons into
   /// the blocks of their users.
public:
   }
 
   /// Return true if the target supports that a subset of CSRs for the given
   /// machine function is handled explicitly via copies.
   virtual bool supportSplitCSR(MachineFunction *MF) const {
     return false;
   }
 
-  /// Return true if the MachineFunction contains a COPY which would imply
-  /// HasCopyImplyingStackAdjustment.
-  virtual bool hasCopyImplyingStackAdjustment(MachineFunction *MF) const {
-    return false;
-  }
-
   /// Perform necessary initialization to handle a subset of CSRs explicitly
   /// via copies. This function is called at the beginning of instruction
   /// selection.
   virtual void initializeSplitCSR(MachineBasicBlock *Entry) const {
     llvm_unreachable("Not Implemented");
   }
 
   /// Insert explicit copies in entry and exit blocks. We copy a subset of

llvm/trunk/lib/CodeGen/GlobalISel/IRTranslator.cpp

for (const Instruction &Inst: BB) {
         << ": '" << InstStr.str() << "'";
       reportTranslationError(*MF, *TPC, *ORE, R);
       return false;
     }
   }
 
   finishPendingPhis();
 
-  // Now that the MachineFrameInfo has been configured, no further changes to
-  // the reserved registers are possible.
-  MRI->freezeReservedRegs(*MF);
+  TLI->finalizeLowering(*MF);
 
   // Merge the argument lowering and constants block with its single
   // successor, the LLVM-IR entry block.  We want the basic block to
   // be maximal.
   assert(EntryBB->succ_size() == 1 &&
          "Custom BB used for lowering should have only one successor");
   // Get the successor of the current entry block.
   MachineBasicBlock &NewEntryBB = **EntryBB->succ_begin();

llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp

for (DenseMap<unsigned, unsigned>::iterator
     // Replacing one register with another won't touch the kill flags.
     // We need to conservatively clear the kill flags as a kill on the old
     // register might dominate existing uses of the new register.
     if (!MRI.use_empty(To))
       MRI.clearKillFlags(From);
     MRI.replaceRegWith(From, To);
   }
 
-  if (TLI->hasCopyImplyingStackAdjustment(MF))
-    MFI.setHasCopyImplyingStackAdjustment(true);
-
-  // Freeze the set of reserved registers now that MachineFrameInfo has been
-  // set up. All the information required by getReservedRegs() should be
-  // available now.
-  MRI.freezeReservedRegs(*MF);
+  TLI->finalizeLowering(*MF);
 
   // Release function-specific state. SDB and CurDAG are already cleared
   // at this point.
   FuncInfo->clear();
 
   DEBUG(dbgs() << "*** MachineFunction at end of ISel ***\n");
   DEBUG(MF->print(dbgs()));
 

llvm/trunk/lib/CodeGen/TargetLoweringBase.cpp

 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/CodeGen/Analysis.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/MachineJumpTableInfo.h"
+#include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/StackMaps.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/Mangler.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCExpr.h"
int TargetLoweringBase::getSqrtRefinementSteps(EVT VT,
                                                MachineFunction &MF) const {
   return getOpRefinementSteps(true, VT, getRecipEstimateForFunc(MF));
 }
 
 int TargetLoweringBase::getDivRefinementSteps(EVT VT,
                                               MachineFunction &MF) const {
   return getOpRefinementSteps(false, VT, getRecipEstimateForFunc(MF));
 }
+
+void TargetLoweringBase::finalizeLowering(MachineFunction &MF) const {
+  MF.getRegInfo().freezeReservedRegs(MF);
+}

llvm/trunk/lib/Target/X86/X86ISelLowering.h

public:
     bool isTypeDesirableForOp(unsigned Opc, EVT VT) const override;
 
     /// Return true if the target has native support for the
     /// specified value type and it is 'desirable' to use the type. e.g. On x86
     /// i16 is legal, but undesirable since i16 instruction encodings are longer
     /// and some i16 instructions are slow.
     bool IsDesirableToPromoteOp(SDValue Op, EVT &PVT) const override;
 
-    /// Return true if the MachineFunction contains a COPY which would imply
-    /// HasOpaqueSPAdjustment.
-    bool hasCopyImplyingStackAdjustment(MachineFunction *MF) const override;
-
     MachineBasicBlock *
     EmitInstrWithCustomInserter(MachineInstr &MI,
                                 MachineBasicBlock *MBB) const override;
 
     /// This method returns the name of a target specific DAG node.
     const char *getTargetNodeName(unsigned Opcode) const override;
 
     bool isCheapToSpeculateCttz() const override;
public:
     unsigned getMaxSupportedInterleaveFactor() const override { return 4; }
 
     /// \brief Lower interleaved load(s) into target specific
     /// instructions/intrinsics.
     bool lowerInterleavedLoad(LoadInst *LI,
                               ArrayRef<ShuffleVectorInst *> Shuffles,
                               ArrayRef<unsigned> Indices,
                               unsigned Factor) const override;
+
+    void finalizeLowering(MachineFunction &MF) const override;
+
   protected:
     std::pair<const TargetRegisterClass *, uint8_t>
     findRepresentativeClass(const TargetRegisterInfo *TRI,
                             MVT VT) const override;
 
   private:
     /// Keep a reference to the X86Subtarget around so that we can
     /// make the right decision when generating code for different targets.

llvm/trunk/lib/Target/X86/X86ISelLowering.cpp

case ISD::XOR:
     return false;
   }
 }
 
 /// This function checks if any of the users of EFLAGS copies the EFLAGS. We
 /// know that the code that lowers COPY of EFLAGS has to use the stack, and if
 /// we don't adjust the stack we clobber the first frame index.
 /// See X86InstrInfo::copyPhysReg.
-bool X86TargetLowering::hasCopyImplyingStackAdjustment(
-    MachineFunction *MF) const {
-  const MachineRegisterInfo &MRI = MF->getRegInfo();
-
+static bool hasCopyImplyingStackAdjustment(const MachineFunction &MF) {
+  const MachineRegisterInfo &MRI = MF.getRegInfo();
   return any_of(MRI.reg_instructions(X86::EFLAGS),
                 [](const MachineInstr &RI) { return RI.isCopy(); });
 }
 
+void X86TargetLowering::finalizeLowering(MachineFunction &MF) const {
+  if (hasCopyImplyingStackAdjustment(MF)) {
+    MachineFrameInfo &MFI = MF.getFrameInfo();
+    MFI.setHasCopyImplyingStackAdjustment(true);
+  }
+
+  TargetLoweringBase::finalizeLowering(MF);
+}
+
 /// This method query the target whether it is beneficial for dag combiner to
 /// promote the specified node. If true, it should return the desired promotion
 /// type by reference.
 bool X86TargetLowering::IsDesirableToPromoteOp(SDValue Op, EVT &PVT) const {
   EVT VT = Op.getValueType();
   if (VT != MVT::i16)
     return false;