[X86] Implement GCC's -mfentry flag

[niravd]

Summary

the -pg flag inserts calls to mcount at the entry of functions in the body for profiling purposes. The -mfentry flag converts such calls into "naked" calls to fentry (in that the call has no prelude/postlude operations).

[niravd]

Ping.

[sanjoy]

Can you please give some context about this: what is __fentry__, mcount, their use cases, relevant specifications?

Also: have you checked to see if some of this code can be shared with XRay?

[niravd]

In D28000#634431, @sanjoy wrote:

Can you please give some context about this: what is __fentry__, mcount, their use cases, relevant specifications?

Updated commit message to reflect this.

Also: have you checked to see if some of this code can be shared with XRay?

I don't think it is (but CC-ing dberris for second opinion). My understanding was that XRay is concerned with function exit and this is concerned with entry.

[niravd]

Ping.

[niravd]

Ping. I believe the only thing holding this up is the potential for sharing overlaps with XRay code, but since this fentry and xray are nominally orthogonal and this deals solely with function entry points we should preserve the logical partitioning as it is.

[niravd]

Ping.

[dberris]

Sorry for the delay -- XRay is interested in both entry and exits. I haven't quite looked into the specifics of this, but it seems like it's doing something very similar to what XRay, or at least the profiling instrumentation code is doing.

[niravd]

All fentry is doing is adding "callq fentry" at the first instruction of every function. It seems unlikely that fentry would ever want to be used with XRay (at the very least it's not obvious who should be on the outside).
We should probably throw a warning/error if they're used together. Do you think it makes sense then to merge the fentry pass into PATCHABLE_FUNCTION_ENTER or leave it as is?

In D28000#653825, @dberris wrote:

Sorry for the delay -- XRay is interested in both entry and exits. I haven't quite looked into the specifics of this, but it seems like it's doing something very similar to what XRay, or at least the profiling instrumentation code is doing.

[dberris]

I'm a little confused here -- you're directly calling a function, before all the stack adjustments are done? I'm sure I understand what this is meant to do, but I'm wondering whether this as written is sufficient (i.e. whether the function being called needs to do the normally caller-saved registers, before doing any work aside from setting up its own stack frame) or "safe".

As far as whether this conflicts with XRay, I'm not really sure it does -- I'd think the XRay instrumentation should come in before the "blind" call to __fentry__ but that's mostly because XRay is meant to measure the cost of running the body of the functions that are instrumented.

Is there a definitive guide to what the flag is supposed to do? And what this supposed to be used for?

[niravd]

In D28000#655790, @dberris wrote:

I'm a little confused here -- you're directly calling a function, before all the stack adjustments are done? I'm sure I understand what this is meant to do, but I'm wondering whether this as written is sufficient (i.e. whether the function being called needs to do the normally caller-saved registers, before doing any work aside from setting up its own stack frame) or "safe".

Right. My understanding is that this is something that was added to gcc 4.6 to help debug in kernel-esque environments. Specifically the goal was to do a mcount-like call before the function prologue. I haven't found a definitive guide though.

The change itself seems straightforward in that it's just mimicking GCC. The only question is its relationship with XRay. I see no good justification for merging the passes as they're seem orthogonal. It might make sense to add a check to cause an error if mfentry and xray are used together since they're both should operate at the prologue (This would be in the clang half D28001). This is not currently the case when -pg and -fxray-instrument are both used; we get the XRay sled before the prologue and mcount afterwards.

[dberris]

In D28000#656376, @niravd wrote:

In D28000#655790, @dberris wrote:

I'm a little confused here -- you're directly calling a function, before all the stack adjustments are done? I'm sure I understand what this is meant to do, but I'm wondering whether this as written is sufficient (i.e. whether the function being called needs to do the normally caller-saved registers, before doing any work aside from setting up its own stack frame) or "safe".

Right. My understanding is that this is something that was added to gcc 4.6 to help debug in kernel-esque environments. Specifically the goal was to do a mcount-like call before the function prologue. I haven't found a definitive guide though.

The change itself seems straightforward in that it's just mimicking GCC. The only question is its relationship with XRay. I see no good justification for merging the passes as they're seem orthogonal. It might make sense to add a check to cause an error if mfentry and xray are used together since they're both should operate at the prologue (This would be in the clang half D28001). This is not currently the case when -pg and -fxray-instrument are both used; we get the XRay sled before the prologue and mcount afterwards.

I think it's fine to keep them separate, I just want to make sure that the XRay sleds are inserted before the mfentry call -- I don't see a good reason to error out in case mfentry and xray are both enabled, they just have to "work as expected". In the same vein -pg and -fxray-instrument ought to work with each other as expected too.

[niravd]

I think it's fine to keep them separate, I just want to make sure that the XRay sleds are inserted before the mfentry call -- I don't see a good reason to error out in case mfentry and xray are both enabled, they just have to "work as expected". In the same vein -pg and -fxray-instrument ought to work with each other as expected too.

Seems good. It's currently in the opposite order, but I'll move the FEntryInserter pass to happen before XRayInstrumentation in TargetPassConfig.cpp.

Dean, can you give me an LGTM on this (and the clang partner diff 28001)?

[dberris]

LGTM

I might be missing it, but do you have the order reversed already?

include/llvm/CodeGen/Passes.h

/// MachineDominanaceFrontier - This pass is a machine dominators analysis pass.
 
   /// \brief This pass lays out funclets contiguously.
   extern char &FuncletLayoutID;
 
   /// This pass inserts the XRay instrumentation sleds if they are supported by
   /// the target platform.
   extern char &XRayInstrumentationID;
 
+  /// This pass inserts FEntry calls
+  extern char &FEntryInserterID;
+
   /// \brief This pass implements the "patchable-function" attribute.
   extern char &PatchableFunctionID;
 
   /// createStackProtectorPass - This pass adds stack protectors to functions.
   ///
   FunctionPass *createStackProtectorPass(const TargetMachine *TM);
 
   /// createMachineVerifierPass - This pass verifies cenerated machine code

include/llvm/InitializePasses.h

 void initializeEdgeBundlesPass(PassRegistry&);
 void initializeEfficiencySanitizerPass(PassRegistry&);
 void initializeEliminateAvailableExternallyLegacyPassPass(PassRegistry &);
 void initializeRAGreedyPass(PassRegistry&);
 void initializeGVNHoistLegacyPassPass(PassRegistry &);
 void initializeExpandISelPseudosPass(PassRegistry&);
 void initializeExpandPostRAPass(PassRegistry&);
 void initializeExternalAAWrapperPassPass(PassRegistry&);
+void initializeFEntryInserterPass(PassRegistry &);
 void initializeFinalizeMachineBundlesPass(PassRegistry&);
 void initializeFlattenCFGPassPass(PassRegistry&);
 void initializeFloat2IntLegacyPassPass(PassRegistry&);
 void initializeForceFunctionAttrsLegacyPassPass(PassRegistry&);
 void initializeForwardControlFlowIntegrityPass(PassRegistry&);
 void initializeFuncletLayoutPass(PassRegistry &);
 void initializeFunctionImportLegacyPassPass(PassRegistry &);
 void initializeGCMachineCodeAnalysisPass(PassRegistry&);

include/llvm/Target/Target.td

 def PATCHABLE_TAIL_CALL : Instruction {
   let OutOperandList = (outs unknown:$dst);
   let InOperandList = (ins variable_ops);
   let AsmString = "# XRay Tail Call Exit.";
   let usesCustomInserter = 1;
   let hasSideEffects = 1;
   let isReturn = 1;
 }
+def FENTRY_CALL : Instruction {
+  let OutOperandList = (outs unknown:$dst);
+  let InOperandList = (ins variable_ops);
+  let AsmString = "# FEntry call";
+  let usesCustomInserter = 1;
+  let mayLoad = 1;
+  let mayStore = 1;
+  let hasSideEffects = 1;
+}
 
 // Generic opcodes used in GlobalISel.
 include "llvm/Target/GenericOpcodes.td"
 
 }
 
 //===----------------------------------------------------------------------===//
 // AsmParser - This class can be implemented by targets that wish to implement

include/llvm/Target/TargetOpcodes.def

 HANDLE_TARGET_OPCODE(LIFETIME_START)
 HANDLE_TARGET_OPCODE(LIFETIME_END)
 
 /// A Stackmap instruction captures the location of live variables at its
 /// position in the instruction stream. It is followed by a shadow of bytes
 /// that must lie within the function and not contain another stackmap.
 HANDLE_TARGET_OPCODE(STACKMAP)
 
+/// FEntry all - This is a marker instruction which gets translated into a raw fentry call.
+HANDLE_TARGET_OPCODE(FENTRY_CALL)
+
 /// Patchable call instruction - this instruction represents a call to a
 /// constant address, followed by a series of NOPs. It is intended to
 /// support optimizations for dynamic languages (such as javascript) that
 /// rewrite calls to runtimes with more efficient code sequences.
 /// This also implies a stack map.
 HANDLE_TARGET_OPCODE(PATCHPOINT)
 
 /// This pseudo-instruction loads the stack guard value. Targets which need

lib/CodeGen/CMakeLists.txt

add_llvm_library(LLVMCodeGen
   DFAPacketizer.cpp
   DwarfEHPrepare.cpp
   EarlyIfConversion.cpp
   EdgeBundles.cpp
   ExecutionDepsFix.cpp
   ExpandISelPseudos.cpp
   ExpandPostRAPseudos.cpp
   FaultMaps.cpp
+  FEntryInserter.cpp
   FuncletLayout.cpp
   GCMetadata.cpp
   GCMetadataPrinter.cpp
   GCRootLowering.cpp
   GCStrategy.cpp
   GlobalMerge.cpp
   IfConversion.cpp
   ImplicitNullChecks.cpp

lib/CodeGen/CodeGen.cpp

void llvm::initializeCodeGen(PassRegistry &Registry) {
   initializeCountingFunctionInserterPass(Registry);
   initializeDeadMachineInstructionElimPass(Registry);
   initializeDetectDeadLanesPass(Registry);
   initializeDwarfEHPreparePass(Registry);
   initializeEarlyIfConverterPass(Registry);
   initializeExpandISelPseudosPass(Registry);
   initializeExpandPostRAPass(Registry);
   initializeFinalizeMachineBundlesPass(Registry);
+  initializeFEntryInserterPass(Registry);
   initializeFuncletLayoutPass(Registry);
   initializeGCMachineCodeAnalysisPass(Registry);
   initializeGCModuleInfoPass(Registry);
   initializeIfConverterPass(Registry);
   initializeInterleavedAccessPass(Registry);
   initializeLiveDebugVariablesPass(Registry);
   initializeLiveIntervalsPass(Registry);
   initializeLiveStacksPass(Registry);

lib/CodeGen/FEntryInserter.cpp

+//===-- FEntryInsertion.cpp - Patchable prologues for LLVM -------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file edits function bodies to insert fentry calls.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/CodeGen/MachineFunction.h"
+#include "llvm/CodeGen/MachineFunctionPass.h"
+#include "llvm/CodeGen/MachineInstrBuilder.h"
+#include "llvm/CodeGen/Passes.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/Module.h"
+#include "llvm/Target/TargetFrameLowering.h"
+#include "llvm/Target/TargetInstrInfo.h"
+#include "llvm/Target/TargetSubtargetInfo.h"
+//#include "llvm/IR/Type.h"
+
+using namespace llvm;
+
+namespace {
+struct FEntryInserter : public MachineFunctionPass {
+  static char ID; // Pass identification, replacement for typeid
+  FEntryInserter() : MachineFunctionPass(ID) {
+    initializeFEntryInserterPass(*PassRegistry::getPassRegistry());
+  }
+
+  bool runOnMachineFunction(MachineFunction &F) override;
+  MachineFunctionProperties getRequiredProperties() const override {
+    return MachineFunctionProperties().set(
+        MachineFunctionProperties::Property::NoVRegs);
+  }
+};
+}
+
+bool FEntryInserter::runOnMachineFunction(MachineFunction &MF) {
+  const std::string FEntryName =
+      MF.getFunction()->getFnAttribute("fentry-call").getValueAsString();
+  if (FEntryName != "true")
+    return false;
+
+  auto &FirstMBB = *MF.begin();
+  auto &FirstMI = *FirstMBB.begin();
+
+  auto *TII = MF.getSubtarget().getInstrInfo();
+  BuildMI(FirstMBB, FirstMI, FirstMI.getDebugLoc(),
+          TII->get(TargetOpcode::FENTRY_CALL));
+  return true;
+}
+
+char FEntryInserter::ID = 0;
+char &llvm::FEntryInserterID = FEntryInserter::ID;
+INITIALIZE_PASS(FEntryInserter, "fentry-insert", "Insert fentry calls", false,
+                false)

lib/CodeGen/TargetPassConfig.cpp

void TargetPassConfig::addMachinePasses() {
   addPass(&FuncletLayoutID, false);
 
   addPass(&StackMapLivenessID, false);
   addPass(&LiveDebugValuesID, false);
 
   addPass(&XRayInstrumentationID, false);
   addPass(&PatchableFunctionID, false);
 
+  addPass(&FEntryInserterID, false);
+
   AddingMachinePasses = false;
 }
 
 /// Add passes that optimize machine instructions in SSA form.
 void TargetPassConfig::addMachineSSAOptimization() {
   // Pre-ra tail duplication.
   addPass(&EarlyTailDuplicateID);
 

lib/Target/X86/X86AsmPrinter.h

class LLVM_LIBRARY_VISIBILITY X86AsmPrinter : public AsmPrinter {
   void LowerTlsAddr(X86MCInstLower &MCInstLowering, const MachineInstr &MI);
 
   // XRay-specific lowering for X86.
   void LowerPATCHABLE_FUNCTION_ENTER(const MachineInstr &MI,
                                      X86MCInstLower &MCIL);
   void LowerPATCHABLE_RET(const MachineInstr &MI, X86MCInstLower &MCIL);
   void LowerPATCHABLE_TAIL_CALL(const MachineInstr &MI, X86MCInstLower &MCIL);
 
+  void LowerFENTRY_CALL(const MachineInstr &MI, X86MCInstLower &MCIL);
+
   // Helper function that emits the XRay sleds we've collected for a particular
   // function.
   void EmitXRayTable();
 
 public:
   explicit X86AsmPrinter(TargetMachine &TM,
                          std::unique_ptr<MCStreamer> Streamer)
       : AsmPrinter(TM, std::move(Streamer)), SM(*this), FM(*this) {}

lib/Target/X86/X86MCInstLower.cpp

for (auto I = MI.operands_begin() + LoadOperandsBeginIdx,
             E = MI.operands_end();
        I != E; ++I)
     if (auto MaybeOperand = MCIL.LowerMachineOperand(&MI, *I))
       LoadMI.addOperand(MaybeOperand.getValue());
 
   OutStreamer->EmitInstruction(LoadMI, getSubtargetInfo());
 }
 
+void X86AsmPrinter::LowerFENTRY_CALL(const MachineInstr &MI,
+                                     X86MCInstLower &MCIL) {
+  bool Is64Bits = Subtarget->is64Bit();
+  MCContext &Ctx = OutStreamer->getContext();
+  MCSymbol *fentry = Ctx.getOrCreateSymbol("__fentry__");
+  const MCSymbolRefExpr *Op =
+      MCSymbolRefExpr::create(fentry, MCSymbolRefExpr::VK_None, Ctx);
+
+  EmitAndCountInstruction(
+      MCInstBuilder(Is64Bits ? X86::CALL64pcrel32 : X86::CALLpcrel32)
+          .addExpr(Op));
+}
+
 void X86AsmPrinter::LowerPATCHABLE_OP(const MachineInstr &MI,
                                       X86MCInstLower &MCIL) {
   // PATCHABLE_OP minsize, opcode, operands
 
   unsigned MinSize = MI.getOperand(0).getImm();
   unsigned Opcode = MI.getOperand(1).getImm();
 
   MCInst MCI;
case X86::ADD32ri: {
     return;
   }
   case TargetOpcode::STATEPOINT:
     return LowerSTATEPOINT(*MI, MCInstLowering);
 
   case TargetOpcode::FAULTING_LOAD_OP:
     return LowerFAULTING_LOAD_OP(*MI, MCInstLowering);
 
+  case TargetOpcode::FENTRY_CALL:
+    return LowerFENTRY_CALL(*MI, MCInstLowering);
+
   case TargetOpcode::PATCHABLE_OP:
     return LowerPATCHABLE_OP(*MI, MCInstLowering);
 
   case TargetOpcode::STACKMAP:
     return LowerSTACKMAP(*MI);
 
   case TargetOpcode::PATCHPOINT:
     return LowerPATCHPOINT(*MI, MCInstLowering);

test/CodeGen/X86/fentry-insertion.ll

+; RUN: llc %s -o - | FileCheck %s
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @test1() #0 {
+entry:
+  ret void
+
+; CHECK-LABEL: @test1
+; CHECK: callq __fentry__
+; CHECK-NOT: mcount
+; CHECK: retq
+}
+
+attributes #0 = { "fentry-call"="true" }
+