[MC] Function stack size section.

[seaneveson]

Summary

Original RFC: http://lists.llvm.org/pipermail/llvm-dev/2017-August/117028.html

I wasn't sure who to put as reviewers, so please add/remove people as appropriate.

This change adds a '.stack-size' section containing metadata on function stack sizes to output ELF files behind the new -stack-size-section flag. The section contains pairs of function symbol references (8 byte) and stack sizes (unsigned LEB128).

The contents of this section can be used to measure changes to stack sizes between different versions of the compiler or a source base. The advantage of having a section is that we can extract this information when examining binaries that we didn't build, and it allows users and tools easy access to that information just by referencing the binary.

There is a follow up change to add an option to clang.

Thanks.

[seaneveson]

Add context to diff. The arc tool doesn't seem to do that for me anymore.

[seaneveson]

Ping.

[MatzeB]

I'd defer to others to comment on the highlevel whether this is a good or helpful idea. Some comments though:

• The format of the new section ought to be documented properly in the llvm docs and not just in the commit message and sourcecode.
• I think this should report a special "unknown" value for functions with dynamic stack allocations.
• I'm not 100% sure that MachineFrameInfo::getStackSize() captures all possible allocations; it probably corresponds to what is allocated in the prologue), however I wonder if we can have cases where callframes are not allocated as part of the prologue. Probably need a stronger wording on what "StackSize" in MachineFrameInfo means if you want to output it to the user in this form.

[MatzeB]

In D39788#926398, @MatzeB wrote:

I'd defer to others to comment on the highlevel whether this is a good or helpful idea. Some comments though:

• The format of the new section ought to be documented properly in the llvm docs and not just in the commit message and sourcecode.
• I think this should report a special "unknown" value for functions with dynamic stack allocations.
• I'm not 100% sure that MachineFrameInfo::getStackSize() captures all possible allocations; it probably corresponds to what is allocated in the prologue), however I wonder if we can have cases where callframes are not allocated as part of the prologue. Probably need a stronger wording on what "StackSize" in MachineFrameInfo means if you want to output it to the user in this form.

To spin this more positively: There seems to be interest for the feature, the code changes appear minimal so maintenance shouldn't be too bad. So I wouldn't object going ahead with this.

However we have to be very careful in managing expectations here! I would expect this to need several iterations, bugfixes and testing/fuzzing until I would trust this information in all cases. Nothing is stopping targets from emitting stack pointer modifying code without recording information in MachineFrameInfo.

[seaneveson]

Thanks for the feedback!

The format of the new section ought to be documented properly in the llvm docs and not just in the commit message and sourcecode.

OK.

I think this should report a special "unknown" value for functions with dynamic stack allocations.

I agree that would be useful. With the value being ULEB128 we could have 0 as "unknown" and encode everything else as value + 1, or we could not emit those functions (Neither is ideal).

I'm not 100% sure that MachineFrameInfo::getStackSize() captures all possible allocations; it probably corresponds to what is allocated in the prologue

Yes I think that is the case.

However we have to be very careful in managing expectations here! I would expect this to need several iterations, bugfixes and testing/fuzzing until I would trust this information in all cases. Nothing is stopping targets from emitting stack pointer modifying code without recording information in MachineFrameInfo.

Agreed. We would like to produce the maximum size each function might be on the stack (where possible). I imagine there will be a lot of cases to account for and we won't be able to foresee all of them (especially for all targets).

[seaneveson]

• Add documentation to the llc CommandGuide
• Skip functions with dynamic stack allocation

[seaneveson]

Ping.

I'd defer to others to comment on the highlevel whether this is a good or helpful idea.

Does anyone have any feedback on that front?

[MatzeB]

docs/CommandGuide/llc.rst

135–141 ↗

(On Diff #123330)

I'd rather move the documentation to a new (sub)section in CodeGenerator.rst as the main purpose would be to document the name and format of the new section.

Documenting the llc flag in addition to that is fine too of course.

lib/MC/MCObjectFileInfo.cpp

597–598

I'm no expert with ELF, but PROGBITS feels wrong to me as I would expect it to get loaded/memory mapped. Is NOTE a better fit?

test/CodeGen/X86/stack-size-section.ll

4–15

I'd move the CHECK lines in front of the functions.

[seaneveson]

• Rebase
• Add documentation to CodeGenerator.rst
• Move test checks above corresponding functions

[seaneveson]

lib/MC/MCObjectFileInfo.cpp

597–598

PROGBITS sections are only loaded when the SHF_ALLOC attribute flag is set, for example the .comment and .debug* sections are PROGBITS.

NOTE has a specific format (array of 4-byte words), which this section doesn't meet.

[MatzeB]

Let's go ahead with this and fix problems as we go along. LGTM, thanks.

include/llvm/CodeGen/AsmPrinter.h

 
   void emitFrameAlloc(const MachineInstr &MI);
 
+  void emitStackSizeSection(const MachineFunction &MF);
+
   enum CFIMoveType { CFI_M_None, CFI_M_EH, CFI_M_Debug };
   CFIMoveType needsCFIMoves() const;
 

include/llvm/CodeGen/CommandFlags.h

                       clEnumValN(DebuggerKind::SCE, "sce",
                                  "SCE targets (e.g. PS4)")));
 
+cl::opt<bool> EnableStackSizeSection(
+    "stack-size-section",
+    cl::desc("Emit a section containing stack size metadata"), cl::init(false));
+
 // Common utility function tightly tied to the options listed here. Initializes
 // a TargetOptions object with CodeGen flags and returns it.
 static inline TargetOptions InitTargetOptionsFromCodeGenFlags() {

   Options.UniqueSectionNames = UniqueSectionNames;
   Options.EmulatedTLS = EmulatedTLS;
   Options.ExceptionModel = ExceptionModel;
+  Options.EmitStackSizeSection = EnableStackSizeSection;
 
   Options.MCOptions = InitMCTargetOptionsFromFlags();
 

include/llvm/MC/MCObjectFileInfo.h

   /// It is initialized on demand so it can be overwritten (with uniquing).
   MCSection *EHFrameSection;
 
+  /// Section containing metadata on function stack sizes.
+  MCSection *StackSizesSection;
+
   // ELF specific sections.
   MCSection *DataRelROSection;
   MCSection *MergeableConst4Section;

   MCSection *getStackMapSection() const { return StackMapSection; }
   MCSection *getFaultMapSection() const { return FaultMapSection; }
 
+  MCSection *getStackSizesSection() const { return StackSizesSection; }
+
   // ELF specific sections.
   MCSection *getDataRelROSection() const { return DataRelROSection; }
   const MCSection *getMergeableConst4Section() const {

include/llvm/Target/TargetOptions.h

           DisableIntegratedAS(false), RelaxELFRelocations(false),
           FunctionSections(false), DataSections(false),
           UniqueSectionNames(true), TrapUnreachable(false), EmulatedTLS(false),
-          EnableIPRA(false) {}
+          EnableIPRA(false), EmitStackSizeSection(false) {}
 
     /// PrintMachineCode - This flag is enabled when the -print-machineinstrs
     /// option is specified on the command line, and should enable debugging

     /// This flag enables InterProcedural Register Allocation (IPRA).
     unsigned EnableIPRA : 1;
 
+    /// Emit section containing metadata on function stack sizes.
+    unsigned EmitStackSizeSection : 1;
+
     /// FloatABIType - This setting is set by -float-abi=xxx option is specfied
     /// on the command line. This setting may either be Default, Soft, or Hard.
     /// Default selects the target's default behavior. Soft selects the ABI for

lib/CodeGen/AsmPrinter/AsmPrinter.cpp

                              MCConstantExpr::create(FrameOffset, OutContext));
 }
 
+void AsmPrinter::emitStackSizeSection(const MachineFunction &MF) {
+  if (!MF.getTarget().Options.EmitStackSizeSection)
+    return;
+
+  MCSection *StackSizeSection = getObjFileLowering().getStackSizesSection();
+  if (!StackSizeSection)
+    return;
+
+  OutStreamer->PushSection();
+  OutStreamer->SwitchSection(StackSizeSection);
+
+  const MCSymbol *FunctionSymbol = getSymbol(MF.getFunction());
+  uint64_t StackSize = MF.getFrameInfo().getStackSize();
+  OutStreamer->EmitValue(MCSymbolRefExpr::create(FunctionSymbol, OutContext),
+                         /* size = */ 8);
+  OutStreamer->EmitULEB128IntValue(StackSize);
+
+  OutStreamer->PopSection();
+}
+
 static bool needFuncLabelsForEHOrDebugInfo(const MachineFunction &MF,
                                            MachineModuleInfo *MMI) {
   if (!MF.getLandingPads().empty() || MF.hasEHFunclets() || MMI->hasDebugInfo())

     HI.Handler->endFunction(MF);
   }
 
+  // Emit section containing stack size metadata.
+  emitStackSizeSection(*MF);
+
   if (isVerbose())
     OutStreamer->GetCommentOS() << "-- End function\n";
 

lib/MC/MCObjectFileInfo.cpp

 
   EHFrameSection =
       Ctx->getELFSection(".eh_frame", EHSectionType, EHSectionFlags);
+
+  StackSizesSection = Ctx->getELFSection(".stack_sizes", ELF::SHT_PROGBITS, 0);
 }
 
 void MCObjectFileInfo::initCOFFMCObjectFileInfo(const Triple &T) {

test/CodeGen/X86/stack-size-section.ll

+; RUN: llc < %s -mtriple=x86_64-linux -stack-size-section | FileCheck %s
+
+; CHECK-LABEL: func1:
+; CHECK: .section .stack_sizes,"",@progbits
+; CHECK-NEXT: .quad func1
+; CHECK-NEXT: .byte 8
+
+; CHECK-LABEL: func2:
+; CHECK: .section .stack_sizes,"",@progbits
+; CHECK-NEXT: .quad func2
+; CHECK-NEXT: .byte 24
+
+define void @func1(i32, i32) #0 {
+  alloca i32, align 4
+  alloca i32, align 4
+  ret void
+}
+
+define void @func2() #1 {
+  alloca i32, align 4
+  call void @func1(i32 1, i32 2)
+  ret void
+}
+
+attributes #0 = { "no-frame-pointer-elim"="true" }
+attributes #1 = { "no-frame-pointer-elim"="true" }