[IPRA] Set callee saved registers to none for local function when IPRA is enabled.

[vivekvpandya]

Summary

IPRA try to optimize caller saved register by propagating register usage information from callee to caller so it is beneficial to have caller saved registers compare to callee saved registers when IPRA is enabled. Please find more detailed explanation here https://groups.google.com/d/msg/llvm-dev/XRzGhJ9wtZg/tjAJqb0eEgAJ.
This change makes local function do not have any callee preserved register when IPRA is enabled. A simple test case is also added to verify this change.

[MatzeB]

lib/CodeGen/TargetFrameLoweringImpl.cpp

28–29

This should go into a header.

63–68

I would first test for if (UseIPRA) and only then do the MF.getFunction(), hasLocalLnkage() && !hasAddressTaken().

test/CodeGen/X86/ipra-local-linkage.ll

5–6

You can probably simplify this further by leaving out the datalayout and using target triple = "x86_64--"

8

The metadata #0 referenced here doesn't exist, I am surprised this even parses...

[vivekvpandya]

test/CodeGen/X86/ipra-local-linkage.ll

8

oh sorry that is there due to copy paste from similar earlier test case.

[vivekvpandya]

Changes as per reviews.

[MatzeB]

include/llvm/Target/TargetFrameLowering.h

21–22

This thing is defined TargetPassConfig.cpp so I would rather expect the declaration in TargetPassConfig.h

test/CodeGen/X86/ipra-local-linkage.ll

9

Do you actually need the nounwind attribute here, I would assume you can just remove it.

[vivekvpandya]

Few more changes as per reviews.

[vivekvpandya]

After this patch test-suite has some runtime failures. Need to work on this.

[vivekvpandya]

The bug related to tail calls and recursive function is solved. The test case is updated as per discussion with @qcolombet. A statistic added to count number of function optimized for not having callee saved registers.

[mehdi_amini]

include/llvm/CodeGen/TargetPassConfig.h

23

I'm not sure it is fine to have a cl::opt in such a global header? If we need the option available broadly we may have to make it a member of TargetPassConfig initialized from a private cl::opt.
(I haven't look in detail where it is needed though).

include/llvm/Target/TargetFrameLowering.h

350

Why not looking at the users of F?

lib/CodeGen/RegUsageInfoCollector.cpp

123

You created F but didn't update uses of MF.getFunction()

129

Add the function name in the debug output.

lib/CodeGen/TargetFrameLoweringImpl.cpp

76

if (UseIPRA && isSafeForNoCSROpt(MF.getFunction()))
  return;

test/CodeGen/X86/ipra-local-linkage.ll

8

Hasn't been fixed apparently.

19

Take %X by argument?

22

saved before and reloaded after?

[vivekvpandya]

test/CodeGen/X86/ipra-local-linkage.ll

19

Do you mean as reference?

22

yes value should be saved if register containing original value is also getting clobbered with function call to foo() but here R15 is having immediate value so it is just getting reloaded.

[vivekvpandya]

test/CodeGen/X86/ipra-local-linkage.ll

8

Yes, it has fixed, Sorry that come again as I modified an older version of it while doign git reset --soft ...

[vivekvpandya]

include/llvm/CodeGen/TargetPassConfig.h

23

UseIPRA is used in TargetPassConfig and TargetFrameLoweringImpl

[vivekvpandya]

test/CodeGen/X86/ipra-local-linkage.ll

22

But yes I will edit comment accordingly to explain it clearly.

[vivekvpandya]

include/llvm/CodeGen/TargetPassConfig.h

23

@MatzeB could you please provide some inputs?

[mehdi_amini]

include/llvm/Target/TargetFrameLowering.h

350

Why has this been marked as done?

lib/CodeGen/RegUsageInfoCollector.cpp

123

Why has this been marked as done?

129

Why has this been marked as done?

lib/CodeGen/TargetFrameLoweringImpl.cpp

76

Why has this been marked as done?

test/CodeGen/X86/ipra-local-linkage.ll

19

I mean as a function parameter, unless I'm missing why an addition is needed here.

[vivekvpandya]

include/llvm/Target/TargetFrameLowering.h

350

Changes have been done on local machine, just waiting for your decision on if UseIPRA to be in TargetPassConfig.h or not?

test/CodeGen/X86/ipra-local-linkage.ll

19

Yes addition is not needed I can change load constant value to variable X.

[mehdi_amini]

include/llvm/Target/TargetFrameLowering.h

350

Please mark as done when the revision is updated, otherwise it makes it harder to track.

test/CodeGen/X86/ipra-local-linkage.ll

19

Why do you need a load? Why don't you take X as a function parameter (you haven't address this question...)

[vivekvpandya]

test/CodeGen/X86/ipra-local-linkage.ll

19

Do you mean as a function parameter of bar() ?

[vivekvpandya]

test/CodeGen/X86/ipra-local-linkage.ll

19

Yes, that also worked. However I have just never thought to use that. But I guess that simplified the test further. In previous version constant value and add instruction were just irrelevant.

[vivekvpandya]

changes as per reviews.

[vivekvpandya]

If this seems good then can someone commit this?

[mehdi_amini]

LGTM.

[mehdi_amini]

Please rebase.

[vivekvpandya]

I did git pull and this patch applied successfully with out any conflicts.

include/llvm/CodeGen/TargetPassConfig.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CODEGEN_TARGETPASSCONFIG_H
 #define LLVM_CODEGEN_TARGETPASSCONFIG_H
 
 #include "llvm/Pass.h"
 #include "llvm/Support/CodeGen.h"
+#include "llvm/Support/CommandLine.h"
 #include <string>
 
+extern llvm::cl::opt<bool> UseIPRA;
+
 namespace llvm {
 
 class PassConfigImpl;
 class ScheduleDAGInstrs;
 class TargetMachine;
 struct MachineSchedContext;
 
 // The old pass manager infrastructure is hidden in a legacy namespace now.

include/llvm/Target/TargetFrameLowering.h

 
 #ifndef LLVM_TARGET_TARGETFRAMELOWERING_H
 #define LLVM_TARGET_TARGETFRAMELOWERING_H
 
 #include "llvm/CodeGen/MachineBasicBlock.h"
 #include <utility>
 #include <vector>
 
 namespace llvm {
   class BitVector;
   class CalleeSavedInfo;
   class MachineFunction;
   class RegScavenger;
 
 /// Information about stack frame layout on the target.  It holds the direction
 /// of stack growth, the known stack alignment on entry to each function, and
 /// the offset to the locals area.
 ///
public:
   /// This method is used by the shrink-wrapping pass to decide if
   /// \p MBB will be correctly handled by the target.
   /// As soon as the target enable shrink-wrapping without overriding
   /// this method, we assume that each basic block is a valid
   /// epilogue.
   virtual bool canUseAsEpilogue(const MachineBasicBlock &MBB) const {
     return true;
   }
+
+  /// Check if given function is safe for not having callee saved registers.
+  /// This is used when interprocedural register allocation is enabled.
+  static bool isSafeForNoCSROpt(const Function *F) {
+    if (!F->hasLocalLinkage() || F->hasAddressTaken() ||
+        !F->hasFnAttribute(Attribute::NoRecurse))
+      return false;
+
+    // Function should not be optimized as tail call.
+    const Module *M = F->getParent();
+    for (const Function &Fu : *M)
+      for (const BasicBlock &BB : Fu)
+        for (const Instruction &II : BB) {
+          if (auto CS = ImmutableCallSite(&II))
+            if (CS.getCalledFunction() == F && CS.isTailCall())
+              return false;
+        }
+
+    return true;
+  }
 };
 
 } // End llvm namespace
 
 #endif

lib/CodeGen/RegUsageInfoCollector.cpp

 ///
 /// This pass is simple MachineFunction pass which collects register usage
 /// details by iterating through each physical registers and checking
 /// MRI::isPhysRegUsed() then creates a RegMask based on this details.
 /// The pass then stores this RegMask in PhysicalRegisterUsageInfo.cpp
 ///
 //===----------------------------------------------------------------------===//
 
+#include "llvm/ADT/Statistic.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/MachineInstr.h"
 #include "llvm/CodeGen/MachineOperand.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/Passes.h"
 #include "llvm/CodeGen/RegisterUsageInfo.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Target/TargetFrameLowering.h"
 
 using namespace llvm;
 
 #define DEBUG_TYPE "ip-regalloc"
 
+STATISTIC(NumCSROpt,
+          "Number of functions optimized for callee saved registers");
+
 namespace llvm {
 void initializeRegUsageInfoCollectorPass(PassRegistry &);
 }
 
 namespace {
 class RegUsageInfoCollector : public MachineFunctionPass {
 public:
   RegUsageInfoCollector() : MachineFunctionPass(ID) {
bool RegUsageInfoCollector::runOnMachineFunction(MachineFunction &MF) {
   std::vector<uint32_t> RegMask;
 
   // Compute the size of the bit vector to represent all the registers.
   // The bit vector is broken into 32-bit chunks, thus takes the ceil of
   // the number of registers divided by 32 for the size.
   unsigned RegMaskSize = (TRI->getNumRegs() + 31) / 32;
   RegMask.resize(RegMaskSize, 0xFFFFFFFF);
 
+  const Function *F = MF.getFunction();
+
   PhysicalRegisterUsageInfo *PRUI = &getAnalysis<PhysicalRegisterUsageInfo>();
 
   PRUI->setTargetMachine(&TM);
 
   DEBUG(dbgs() << "Clobbered Registers: ");
 
   for (unsigned PReg = 1, PRegE = TRI->getNumRegs(); PReg < PRegE; ++PReg)
     if (MRI->isPhysRegModified(PReg, true))
       markRegClobbered(TRI, &RegMask[0], PReg);
 
+  if (!TargetFrameLowering::isSafeForNoCSROpt(F)) {
     const uint32_t *CallPreservedMask =
         TRI->getCallPreservedMask(MF, MF.getFunction()->getCallingConv());
     // Set callee saved register as preserved.
     for (unsigned i = 0; i < RegMaskSize; ++i)
       RegMask[i] = RegMask[i] | CallPreservedMask[i];
+  } else {
+    ++NumCSROpt;
+    DEBUG(dbgs() << "Function optimized for not having CSR.\n");
+  }
 
   for (unsigned PReg = 1, PRegE = TRI->getNumRegs(); PReg < PRegE; ++PReg)
     if (MachineOperand::clobbersPhysReg(&(RegMask[0]), PReg))
       DEBUG(dbgs() << TRI->getName(PReg) << " ");
 
   DEBUG(dbgs() << " \n----------------------------------------\n");
 
   PRUI->storeUpdateRegUsageInfo(MF.getFunction(), std::move(RegMask));
 
   return false;
 }

lib/CodeGen/TargetFrameLoweringImpl.cpp

 //===----- TargetFrameLoweringImpl.cpp - Implement target frame interface --==//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // Implements the layout of a stack frame on the target machine.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/BitVector.h"
-#include "llvm/Target/TargetFrameLowering.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
+#include "llvm/CodeGen/TargetPassConfig.h"
 #include "llvm/IR/CallingConv.h"
 #include "llvm/IR/Function.h"
+#include "llvm/Target/TargetFrameLowering.h"
 #include "llvm/Target/TargetRegisterInfo.h"
 #include "llvm/Target/TargetSubtargetInfo.h"
 #include <cstdlib>
 using namespace llvm;
 
 TargetFrameLowering::~TargetFrameLowering() {
 }
 
 /// The default implementation just looks at attribute "no-frame-pointer-elim".
 bool TargetFrameLowering::noFramePointerElim(const MachineFunction &MF) const {
   auto Attr = MF.getFunction()->getFnAttribute("no-frame-pointer-elim");
   return Attr.getValueAsString() == "true";
 }
 
 /// Returns the displacement from the frame register to the stack

 bool TargetFrameLowering::needsFrameIndexResolution(
     const MachineFunction &MF) const {
   return MF.getFrameInfo()->hasStackObjects();
 }
 
 void TargetFrameLowering::determineCalleeSaves(MachineFunction &MF,
                                                BitVector &SavedRegs,
                                                RegScavenger *RS) const {
-  // Get the callee saved register list...
   const TargetRegisterInfo &TRI = *MF.getSubtarget().getRegisterInfo();
-  const MCPhysReg *CSRegs = TRI.getCalleeSavedRegs(&MF);
 
   // Resize before the early returns. Some backends expect that
   // SavedRegs.size() == TRI.getNumRegs() after this call even if there are no
   // saved registers.
   SavedRegs.resize(TRI.getNumRegs());
 
+  // When interprocedural register allocation is enabled caller saved registers
+  // are preferred over callee saved registers.
+  if (UseIPRA) {
+    const Function *F = MF.getFunction();
+    if (isSafeForNoCSROpt(F))
+      return;
+  }
+
+  // Get the callee saved register list...
+  const MCPhysReg *CSRegs = TRI.getCalleeSavedRegs(&MF);
+
   // Early exit if there are no callee saved registers.
   if (!CSRegs || CSRegs[0] == 0)
     return;
 
   // In Naked functions we aren't going to save any registers.
   if (MF.getFunction()->hasFnAttribute(Attribute::Naked))
     return;
 

lib/CodeGen/TargetPassConfig.cpp

 #include "llvm/Analysis/TypeBasedAliasAnalysis.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/RegAllocRegistry.h"
 #include "llvm/CodeGen/RegisterUsageInfo.h"
 #include "llvm/IR/IRPrintingPasses.h"
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/MC/MCAsmInfo.h"
-#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Transforms/Instrumentation.h"
 #include "llvm/Transforms/Scalar.h"
 #include "llvm/Transforms/Utils/SymbolRewriter.h"
 

test/CodeGen/X86/ipra-local-linkage.ll

+; RUN: llc < %s | FileCheck %s -check-prefix=NOIPRA
+; RUN: llc -enable-ipra < %s | FileCheck %s
+
+target triple = "x86_64--"
+
+define internal void @foo() norecurse #0 {
+; When IPRA is not enabled R15 will be saved by foo as it is callee saved reg.
+; NOIPRA-LABEL: foo:
+; NOIPRA: pushq %r15
+; When IPRA is enabled none register should be saved as foo() is local function
+; so we optimize it to save no registers.
+; CHECK-LABEL: foo:
+; CHECK-NOT: pushq %r15
+  call void asm sideeffect "movl  %r14d, %r15d", "~{r15}"() #0
+  ret void
+}
+
+define void @bar() #0 {
+  %X = add i32 1, 3
+  call void asm sideeffect "movl  %r12d, $0", "{r15}~{r12}"(i32 %X) #0
+  ; As R15 is clobbered by foo() when IPRA is enabled value of R15 should be
+  ; reloaded after foo()
+  call void @foo()
+  ; CHECK-LABEL: bar:
+  ; CHECK: callq foo
+  ; CHECK-NEXT: movl  $4, %r15d
+  call void asm sideeffect "movl  $0, %r12d", "{r15}~{r12}"(i32 %X) #0
+  ret void
+}