[MachineOutliner] Add IsOutlined to MachineFunction

[• pcwang-thead]

Summary

We add a field IsOutlined to indicate whether a MachineFunction
is outlined and set it true for outlined functions in MachineOutliner.

[craig.topper]

llvm/include/llvm/CodeGen/MachineFunction.h

377

If we add this property, it needs to be connected to MIR parsing and printing.

[jrtc27]

Is there a reason we can't instead add the right live-outs?

[• pcwang-thead]

In D146191#4201151, @jrtc27 wrote:

Is there a reason we can't instead add the right live-outs?

I tried, but it seems to be hard.
We call these outlined functions as function, but they are just shared code fragments with only one basic block. There are some differences between normal functions and outlined functions:

1. For outlined functions, they end with a JALR instruction (in which isReturn is false); while for normal functions, they end with a PseudoRET (in which isReturn is true). So the only basic block in outlined function is not ReturnBlock(See MachineBasicBlock::isReturnBlock()).
2. For normal functions, the live-outs of ReturnBlock are callee-saved registers and return value(s)? (IIUC, correct me if I am wrong). But all defs in outlined functions should be live.

I digged into LivePhysRegs.cpp and I could't find a way to add live-outs manually.

[• pcwang-thead]

Add MIR parsing and printing.

[craig.topper]

Do the T-Head specific pseudos being expanded need to expanded as late as the other pseudos? Could you have an earlier expansion pass? I was already nervous about the short forward branch instructions being affected before. I know we determined those cases were ok.

[• pcwang-thead]

In D146191#4225567, @craig.topper wrote:

Do the T-Head specific pseudos being expanded need to expanded as late as the other pseudos? Could you have an earlier expansion pass? I was already nervous about the short forward branch instructions being affected before. I know we determined those cases were ok.

Yes and thanks, I just tried. They can be placed in addPreEmitPass() just like MCP.
Apart from this, do you think it's still worthy marking outlined functions?

[asb]

In D146191#4226495, @pcwang-thead wrote:

In D146191#4225567, @craig.topper wrote:

Do the T-Head specific pseudos being expanded need to expanded as late as the other pseudos? Could you have an earlier expansion pass? I was already nervous about the short forward branch instructions being affected before. I know we determined those cases were ok.

Yes and thanks, I just tried. They can be placed in addPreEmitPass() just like MCP.
Apart from this, do you think it's still worthy marking outlined functions?

For my part, I'd be really interested in @paquette's thoughts on this.

[paquette]

In D146191#4227164, @asb wrote:

In D146191#4226495, @pcwang-thead wrote:

In D146191#4225567, @craig.topper wrote:

Do the T-Head specific pseudos being expanded need to expanded as late as the other pseudos? Could you have an earlier expansion pass? I was already nervous about the short forward branch instructions being affected before. I know we determined those cases were ok.

Yes and thanks, I just tried. They can be placed in addPreEmitPass() just like MCP.
Apart from this, do you think it's still worthy marking outlined functions?

For my part, I'd be really interested in @paquette's thoughts on this.

If you mean adding the IsOutlined field, I think that it's a good idea. Currently the only way to recognize if a function is outlined is to look at its name, which isn't great. The field is an improvement.

I think it needs a small MIR testcase though. :)

[asb]

Thanks Jessica - in that case, LGTM once it has the MIR test case Jessica requested.

[craig.topper]

In D146191#4230864, @asb wrote:

Thanks Jessica - in that case, LGTM once it has the MIR test case Jessica requested.

I thought the earlier conclusion was that the RISC-V part wasn’t needed if T-Head moves their pseudo expansion in their downstream.

[asb]

In D146191#4230950, @craig.topper wrote:

In D146191#4230864, @asb wrote:

Thanks Jessica - in that case, LGTM once it has the MIR test case Jessica requested.

I thought the earlier conclusion was that the RISC-V part wasn’t needed if T-Head moves their pseudo expansion in their downstream.

Yes, let's drop the RISC-V part of this if there's not a good motivation for running MCP so late.

[• pcwang-thead]

Remove MCP parts and add MIR test.

I didn't find a target-independent way to test this, so I add some CHECKs to existed RISCV test.

[• pcwang-thead]

Ping. :-)

[paquette]

LGTM!

llvm/include/llvm/CodeGen/MachineFunction.h

class LLVM_EXTERNAL_VISIBILITY MachineFunction {
   /// CodeView label annotations.
   std::vector<std::pair<MCSymbol *, MDNode *>> CodeViewAnnotations;
 
   bool CallsEHReturn = false;
   bool CallsUnwindInit = false;
   bool HasEHCatchret = false;
   bool HasEHScopes = false;
   bool HasEHFunclets = false;
+  bool IsOutlined = false;
 
   /// BBID to assign to the next basic block of this function.
   unsigned NextBBID = 0;
 
   /// Section Type for basic blocks, only relevant with basic block sections.
   BasicBlockSection BBSectionsType = BasicBlockSection::None;
 
   /// List of C++ TypeInfo used.
public:
   void setHasEHCatchret(bool V) { HasEHCatchret = V; }
 
   bool hasEHScopes() const { return HasEHScopes; }
   void setHasEHScopes(bool V) { HasEHScopes = V; }
 
   bool hasEHFunclets() const { return HasEHFunclets; }
   void setHasEHFunclets(bool V) { HasEHFunclets = V; }
 
+  bool isOutlined() const { return IsOutlined; }
+  void setIsOutlined(bool V) { IsOutlined = V; }
+
   /// Find or create an LandingPadInfo for the specified MachineBasicBlock.
   LandingPadInfo &getOrCreateLandingPadInfo(MachineBasicBlock *LandingPad);
 
   /// Return a reference to the landing pad info for the current function.
   const std::vector<LandingPadInfo> &getLandingPads() const {
     return LandingPads;
   }
 

llvm/lib/CodeGen/MachineCopyPropagation.cpp

void MachineCopyPropagation::EliminateSpillageCopies(MachineBasicBlock &MBB) {
   CopyDbgUsers.clear();
   Tracker.clear();
 }
 
 bool MachineCopyPropagation::runOnMachineFunction(MachineFunction &MF) {
   if (skipFunction(MF.getFunction()))
     return false;
 
+  if (MF.isOutlined())
+    return false;
+
   bool isSpillageCopyElimEnabled = false;
   switch (EnableSpillageCopyElimination) {
   case cl::BOU_UNSET:
     isSpillageCopyElimEnabled =
         MF.getSubtarget().enableSpillageCopyElimination();
     break;
   case cl::BOU_TRUE:
     isSpillageCopyElimEnabled = true;

llvm/lib/CodeGen/MachineOutliner.cpp

if (UW != UWTableKind::None)
     F->setUWTableKind(UW);
 
   BasicBlock *EntryBB = BasicBlock::Create(C, "entry", F);
   IRBuilder<> Builder(EntryBB);
   Builder.CreateRetVoid();
 
   MachineModuleInfo &MMI = getAnalysis<MachineModuleInfoWrapperPass>().getMMI();
   MachineFunction &MF = MMI.getOrCreateMachineFunction(*F);
+  MF.setIsOutlined(true);
   MachineBasicBlock &MBB = *MF.CreateMachineBasicBlock();
 
   // Insert the new function into the module.
   MF.insert(MF.begin(), &MBB);
 
   MachineFunction *OriginalMF = FirstCand.front()->getMF();
   const std::vector<MCCFIInstruction> &Instrs =
       OriginalMF->getFrameInstructions();

llvm/lib/Target/RISCV/RISCVTargetMachine.cpp

bool RISCVPassConfig::addGlobalInstructionSelect() {
   return false;
 }
 
 void RISCVPassConfig::addPreSched2() {}
 
 void RISCVPassConfig::addPreEmitPass() {
   addPass(&BranchRelaxationPassID);
   addPass(createRISCVMakeCompressibleOptPass());
-
-  // TODO: It would potentially be better to schedule copy propagation after
-  // expanding pseudos (in addPreEmitPass2). However, performing copy
-  // propagation after the machine outliner (which runs after addPreEmitPass)
-  // currently leads to incorrect code-gen, where copies to registers within
-  // outlined functions are removed erroneously.
-  if (TM->getOptLevel() >= CodeGenOpt::Default && EnableRISCVCopyPropagation)
-    addPass(createMachineCopyPropagationPass(true));
 }
 
 void RISCVPassConfig::addPreEmitPass2() {
   addPass(createRISCVExpandPseudoPass());
 
+  // Do the copy propagation after expanding pseudos because we may produce some
+  // MVs when expanding.
+  if (TM->getOptLevel() >= CodeGenOpt::Default && EnableRISCVCopyPropagation)
+    addPass(createMachineCopyPropagationPass(true));
+
   // Schedule the expansion of AMOs at the last possible moment, avoiding the
   // possibility for other passes to break the requirements for forward
   // progress in the LR/SC block.
   addPass(createRISCVExpandAtomicPseudoPass());
 }
 
 void RISCVPassConfig::addMachineSSAOptimization() {
   TargetPassConfig::addMachineSSAOptimization();

llvm/test/CodeGen/RISCV/O3-pipeline.ll

 ; CHECK-NEXT:       Machine Block Frequency Analysis
 ; CHECK-NEXT:       MachinePostDominator Tree Construction
 ; CHECK-NEXT:       Branch Probability Basic Block Placement
 ; CHECK-NEXT:       Insert fentry calls
 ; CHECK-NEXT:       Insert XRay ops
 ; CHECK-NEXT:       Implement the 'patchable-function' attribute
 ; CHECK-NEXT:       Branch relaxation pass
 ; CHECK-NEXT:       RISCV Make Compressible
-; CHECK-NEXT:       Machine Copy Propagation Pass
 ; CHECK-NEXT:       Contiguously Lay Out Funclets
 ; CHECK-NEXT:       StackMap Liveness Analysis
 ; CHECK-NEXT:       Live DEBUG_VALUE analysis
 ; CHECK-NEXT:       Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:     Machine Outliner
 ; CHECK-NEXT:     FunctionPass Manager
 ; CHECK-NEXT:       Lazy Machine Block Frequency Analysis
 ; CHECK-NEXT:       Machine Optimization Remark Emitter
 ; CHECK-NEXT:       Stack Frame Layout Analysis
 ; CHECK-NEXT:       RISCV pseudo instruction expansion pass
+; CHECK-NEXT:       Machine Copy Propagation Pass
 ; CHECK-NEXT:       RISCV atomic pseudo instruction expansion pass
 ; CHECK-NEXT:       Lazy Machine Block Frequency Analysis
 ; CHECK-NEXT:       Machine Optimization Remark Emitter
 ; CHECK-NEXT:       RISCV Assembly Printer
 ; CHECK-NEXT:       Free MachineFunction