[CodeGen] MRI call back in TargetMachine

[cdevadas]

Summary

It is needed for target specific initializatons.

[qcolombet]

Hi @cdevadas,

What do you want to achieve with this call back?

It feels wrong to me that the TargetMachine would need to know about the MRI. IIRC, the TargetMachine holds information that applies to the whole target and doesn't do any specific based on each function. (At least it doesn't hold any state.)

Cheers,
-Quentin

[arsenm]

In D143758#4154723, @qcolombet wrote:

Hi @cdevadas,

What do you want to achieve with this call back?

It feels wrong to me that the TargetMachine would need to know about the MRI. IIRC, the TargetMachine holds information that applies to the whole target and doesn't do any specific based on each function. (At least it doesn't hold any state.)

Cheers,
-Quentin

The TargetMachine doesn't maintain state but manages the construction of MachineFunctionInfo and MIR parsing. This change is the consequence of your suggestion to use the callbacks for virtual registers (D134950) instead of the directly tracking virtual register flags in MRI. If we have to handle this in a side table stored in MFI, the TargetMachine needs to register these callbacks at the same time MFI is constructed (which is here).

[qcolombet]

I see, you need to hook up the delegate registration.
Make sense.

Let me think about it.

If anything we probably need better comments to make it more obvious what the purpose of this callback is.

I'll get back to you.

[qcolombet]

Sorry for the delay I don't have an answer, but I have a couple of questions:

1. Would it work to attach the delegate in MachineFunction::initTargetMachineFunctionInfo?
2. Do we want to support MachineFunction/Info cloning?

For #1 I guess it depends whether or not we want to offer a more general hook point than just MachineFunctionInfo.
I am guessing we want to support #2, and #1 doesn't work for that and neither is the current patch. We probably want to add a call to this hook during ::clone.

Anyhow, I am willing to go with the current patch if we add an actual use case in this patch. I'd like that we have an example that goes in at the same time as we introduce registerMachineRegisterCallback.

[arsenm]

In D143758#4242979, @qcolombet wrote:

Sorry for the delay I don't have an answer, but I have a couple of questions:

1. Would it work to attach the delegate in MachineFunction::initTargetMachineFunctionInfo?

Probably

2. Do we want to support MachineFunction/Info cloning?

This has to work and needs to be serializable.

For #1 I guess it depends whether or not we want to offer a more general hook point than just MachineFunctionInfo.
I am guessing we want to support #2, and #1 doesn't work for that and neither is the current patch. We probably want to add a call to this hook during ::clone.

Anyhow, I am willing to go with the current patch if we add an actual use case in this patch. I'd like that we have an example that goes in at the same time as we introduce registerMachineRegisterCallback.

I strongly think this should be kept separate. The actual use patch is much larger and conceptually detached.

[qcolombet]

Do we want to support MachineFunction/Info cloning?

This has to work and needs to be serializable.

What's the plan to support that?

I strongly think this should be kept separate. The actual use patch is much larger and conceptually detached.

Fair enough, I was hoping for something small.

[yassingh]

• Rebase over new TARGET::PRED_COPY approach

[arsenm]

In D143758#4265875, @qcolombet wrote:

Do we want to support MachineFunction/Info cloning?

This has to work and needs to be serializable.

What's the plan to support that?

With the current scheme it's doable in the AMDGPU MFI handling

[yassingh]

Rebase

llvm/include/llvm/Target/TargetMachine.h

public:
   /// default. The -enable-ipra flag can be used to override this.
   virtual bool useIPRA() const {
     return false;
   }
 
   /// The default variant to use in unqualified `asm` instructions.
   /// If this returns 0, `asm "$(foo$|bar$)"` will evaluate to `asm "foo"`.
   virtual int unqualifiedInlineAsmVariant() const { return 0; }
+
+  // MachineRegisterInfo callback function
+  virtual void registerMachineRegisterInfoCallback(MachineFunction &MF) const {}
 };
 
 /// Helper method for getting the code model, returning Default if
 /// CM does not have a value. The tiny and kernel models will produce
 /// an error, so targets that support them or require more complex codemodel
 /// selection logic should implement and call their own getEffectiveCodeModel.
 inline CodeModel::Model
 getEffectiveCodeModel(std::optional<CodeModel::Model> CM,

llvm/lib/CodeGen/MachineModuleInfo.cpp

MachineFunction &MachineModuleInfo::getOrCreateMachineFunction(Function &F) {
   auto I = MachineFunctions.insert(
       std::make_pair(&F, std::unique_ptr<MachineFunction>()));
   MachineFunction *MF;
   if (I.second) {
     // No pre-existing machine function, create a new one.
     const TargetSubtargetInfo &STI = *TM.getSubtargetImpl(F);
     MF = new MachineFunction(F, TM, STI, NextFnNum++, *this);
     MF->initTargetMachineFunctionInfo(STI);
+
+    // MRI callback for target specific initializations.
+    TM.registerMachineRegisterInfoCallback(*MF);
+
     // Update the set entry.
     I.first->second.reset(MF);
   } else {
     MF = I.first->second.get();
   }
 
   LastRequest = &F;
   LastResult = MF;

llvm/lib/CodeGen/ResetMachineFunctionPass.cpp

 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/Passes.h"
 #include "llvm/CodeGen/StackProtector.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Target/TargetMachine.h"
 using namespace llvm;
 
 #define DEBUG_TYPE "reset-machine-function"
 
 STATISTIC(NumFunctionsReset, "Number of functions reset");
 STATISTIC(NumFunctionsVisited, "Number of functions visited");
 
 namespace {
bool runOnMachineFunction(MachineFunction &MF) override {
               MachineFunctionProperties::Property::FailedISel)) {
         if (AbortOnFailedISel)
           report_fatal_error("Instruction selection failed");
         LLVM_DEBUG(dbgs() << "Resetting: " << MF.getName() << '\n');
         ++NumFunctionsReset;
         MF.reset();
         MF.initTargetMachineFunctionInfo(MF.getSubtarget());
 
+        const LLVMTargetMachine &TM = MF.getTarget();
+        // MRI callback for target specific initializations.
+        TM.registerMachineRegisterInfoCallback(MF);
+
         if (EmitFallbackDiag) {
           const Function &F = MF.getFunction();
           DiagnosticInfoISelFallback DiagFallback(F);
           F.getContext().diagnose(DiagFallback);
         }
         return true;
       }
       return false;