[MC] Teach the MachO object writer about N_FUNC_COLD

[vsk]

Summary

N_FUNC_COLD is a new MachO symbol attribute. It's a hint to the linker
to order a symbol towards the end of its section, to improve locality.

Example:

void a1() {}
__attribute__((cold)) void a2() {}
void a3() {}
int main() {
  a1();
  a2();
  a3();
  return 0;
}

A linker that supports N_FUNC_COLD will order _a2 to the end of the text
section. From nm -njU output, we see:

_a1
_a3
_main
_a2

[vsk]

+ Mike, who's touched nm recently.

[mtrent]

llvm-nm and BinaryFormat changes look good to me.

[pcc]

Interesting, what's the advantage of this over doing something like creating a __cold section in the __TEXT segment?

llvm/lib/MC/MCAsmStreamer.cpp

660

It should be possible to teach MC to parse this at least, right?

[vsk]

Per @pcc's suggestion, add basic AsmParser support for the .cold directive.

[vsk]

In D57190#1370535, @pcc wrote:

Interesting, what's the advantage of this over doing something like creating a __cold section in the __TEXT segment?

A .cold directive allows us to perform reordering in projects/build systems we don't control, which may have custom linker scripts, or other tooling which depends on text remaining in the __text section.

[mtrent]

llvm/trunk/tools/llvm-nm/llvm-nm.cpp

560 ↗

(On Diff #183573)

Actually, upon further review, the text should be "[cold func]" to match the name of the symbol.

[vsk]

llvm/trunk/tools/llvm-nm/llvm-nm.cpp

560 ↗

(On Diff #183573)

Done in r352258.

[kledzik]

llvm/trunk/lib/MC/MCAsmStreamer.cpp

660 ↗

(On Diff #183573)

Will there be a follow on patch to add a directive to allow cold attribute on symbols? Otherwise work flows that go through .s files will lose cold information.

[vsk]

llvm/trunk/lib/MC/MCAsmStreamer.cpp

660 ↗

(On Diff #183573)

Yes, I'm tracking this in rdar://47598547 and rdar://47598666.

[thakis]

llvm/trunk/include/llvm/BinaryFormat/MachO.h

337 ↗

(On Diff #183573)

This (and the previous bits too) overlap with the ordinal written by http://llvm-cs.pcc.me.uk/include/llvm/BinaryFormat/MachO.h/rSET_LIBRARY_ORDINAL

I suppose the thinking is that that's safe because the ordinal is only set on undefs while these 3 bits here can't be set on undefs? If so, maybe this warrants a comment?

[vsk]

llvm/trunk/include/llvm/BinaryFormat/MachO.h

337 ↗

(On Diff #183573)

I wasn't aware of the overlap and am not sure I understand whether it's safe. I know N_COLD_FUNC /shouldn't/ be set on undefs -- we may want to assert that -- but am not sure about the other two. @pete & @davide - your thoughts?

llvm/include/llvm/BinaryFormat/MachO.h

enum {
   // Flag bits (some overlap with the library ordinal bits).
   N_ARM_THUMB_DEF = 0x0008u,
   REFERENCED_DYNAMICALLY = 0x0010u,
   N_NO_DEAD_STRIP = 0x0020u,
   N_WEAK_REF = 0x0040u,
   N_WEAK_DEF = 0x0080u,
   N_SYMBOL_RESOLVER = 0x0100u,
   N_ALT_ENTRY = 0x0200u,
+  N_COLD_FUNC = 0x0400u,
   // For undefined symbols coming from libraries, see GET_LIBRARY_ORDINAL()
   // as these are in the top 8 bits.
   SELF_LIBRARY_ORDINAL = 0x0,
   MAX_LIBRARY_ORDINAL = 0xfd,
   DYNAMIC_LOOKUP_ORDINAL = 0xfe,
   EXECUTABLE_ORDINAL = 0xff
 };
 

llvm/include/llvm/MC/MCDirectives.h

 #define LLVM_MC_MCDIRECTIVES_H
 
 namespace llvm {
 
 enum MCSymbolAttr {
   MCSA_Invalid = 0,    ///< Not a valid directive.
 
   // Various directives in alphabetical order.
+  MCSA_Cold,                ///< .cold (MachO)
   MCSA_ELF_TypeFunction,    ///< .type _foo, STT_FUNC  # aka @function
   MCSA_ELF_TypeIndFunction, ///< .type _foo, STT_GNU_IFUNC
   MCSA_ELF_TypeObject,      ///< .type _foo, STT_OBJECT  # aka @object
   MCSA_ELF_TypeTLS,         ///< .type _foo, STT_TLS     # aka @tls_object
   MCSA_ELF_TypeCommon,      ///< .type _foo, STT_COMMON  # aka @common
   MCSA_ELF_TypeNoType,      ///< .type _foo, STT_NOTYPE  # aka @notype
   MCSA_ELF_TypeGnuUniqueObject, /// .type _foo, @gnu_unique_object
   MCSA_Global,              ///< .globl

llvm/include/llvm/MC/MCSymbolMachO.h

enum MachOSymbolFlags : uint16_t { // See <mach-o/nlist.h>.
 
     // Other 'desc' flags.
     SF_ThumbFunc                            = 0x0008,
     SF_NoDeadStrip                          = 0x0020,
     SF_WeakReference                        = 0x0040,
     SF_WeakDefinition                       = 0x0080,
     SF_SymbolResolver                       = 0x0100,
     SF_AltEntry                             = 0x0200,
+    SF_Cold                                 = 0x0400,
 
     // Common alignment
     SF_CommonAlignmentMask                  = 0xF0FF,
     SF_CommonAlignmentShift                 = 8
   };
 
 public:
   MCSymbolMachO(const StringMapEntry<bool> *Name, bool isTemporary)
public:
   void setAltEntry() const {
     modifyFlags(SF_AltEntry, SF_AltEntry);
   }
 
   bool isAltEntry() const {
     return getFlags() & SF_AltEntry;
   }
 
+  void setCold() const { modifyFlags(SF_Cold, SF_Cold); }
+
+  bool isCold() const { return getFlags() & SF_Cold; }
+
   void setDesc(unsigned Value) const {
     assert(Value == (Value & SF_DescFlagsMask) &&
            "Invalid .desc value!");
     setFlags(Value & SF_DescFlagsMask);
   }
 
   /// Get the encoded value of the flags as they will be emitted in to
   /// the MachO binary

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

void AsmPrinter::EmitFunctionHeader() {
 
   EmitLinkage(&F, CurrentFnSym);
   if (MAI->hasFunctionAlignment())
     EmitAlignment(MF->getAlignment(), &F);
 
   if (MAI->hasDotTypeDotSizeDirective())
     OutStreamer->EmitSymbolAttribute(CurrentFnSym, MCSA_ELF_TypeFunction);
 
+  if (F.hasFnAttribute(Attribute::Cold))
+    OutStreamer->EmitSymbolAttribute(CurrentFnSym, MCSA_Cold);
+
   if (isVerbose()) {
     F.printAsOperand(OutStreamer->GetCommentOS(),
                    /*PrintType=*/false, F.getParent());
     OutStreamer->GetCommentOS() << '\n';
   }
 
   // Emit the prefix data.
   if (F.hasPrefixData()) {

llvm/lib/MC/MCAsmStreamer.cpp

bool MCAsmStreamer::EmitSymbolAttribute(MCSymbol *Symbol,
   case MCSA_Reference:      OS << "\t.reference\t";       break;
   case MCSA_Weak:           OS << MAI->getWeakDirective(); break;
   case MCSA_WeakDefinition:
     OS << "\t.weak_definition\t";
     break;
       // .weak_reference
   case MCSA_WeakReference:  OS << MAI->getWeakRefDirective(); break;
   case MCSA_WeakDefAutoPrivate: OS << "\t.weak_def_can_be_hidden\t"; break;
+  case MCSA_Cold:
+    // Assemblers currently do not support a .cold directive.
+    return false;
   }
 
   Symbol->print(OS, MAI);
   EmitEOL();
 
   return true;
 }
 

llvm/lib/MC/MCELFStreamer.cpp

bool MCELFStreamer::EmitSymbolAttribute(MCSymbol *S, MCSymbolAttr Attribute) {
 
   // The implementation of symbol attributes is designed to match 'as', but it
   // leaves much to desired. It doesn't really make sense to arbitrarily add and
   // remove flags, but 'as' allows this (in particular, see .desc).
   //
   // In the future it might be worth trying to make these operations more well
   // defined.
   switch (Attribute) {
+  case MCSA_Cold:
   case MCSA_LazyReference:
   case MCSA_Reference:
   case MCSA_SymbolResolver:
   case MCSA_PrivateExtern:
   case MCSA_WeakDefinition:
   case MCSA_WeakDefAutoPrivate:
   case MCSA_Invalid:
   case MCSA_IndirectSymbol:

llvm/lib/MC/MCMachOStreamer.cpp

case MCSA_WeakDefinition:
     // it has to be in a coalesced section, but this isn't enforced.
     Symbol->setWeakDefinition();
     break;
 
   case MCSA_WeakDefAutoPrivate:
     Symbol->setWeakDefinition();
     Symbol->setWeakReference();
     break;
+
+  case MCSA_Cold:
+    Symbol->setCold();
+    break;
   }
 
   return true;
 }
 
 void MCMachOStreamer::EmitSymbolDesc(MCSymbol *Symbol, unsigned DescValue) {
   // Encode the 'desc' value into the lowest implementation defined bits.
   getAssembler().registerSymbol(*Symbol);

llvm/lib/MC/MCParser/AsmParser.cpp

enum DirectiveKind {
     DK_LAZY_REFERENCE,
     DK_NO_DEAD_STRIP,
     DK_SYMBOL_RESOLVER,
     DK_PRIVATE_EXTERN,
     DK_REFERENCE,
     DK_WEAK_DEFINITION,
     DK_WEAK_REFERENCE,
     DK_WEAK_DEF_CAN_BE_HIDDEN,
+    DK_COLD,
     DK_COMM,
     DK_COMMON,
     DK_LCOMM,
     DK_ABORT,
     DK_INCLUDE,
     DK_INCBIN,
     DK_CODE16,
     DK_CODE16GCC,
if (IDVal.startswith(".") && IDVal != ".") {
     case DK_REFERENCE:
       return parseDirectiveSymbolAttribute(MCSA_Reference);
     case DK_WEAK_DEFINITION:
       return parseDirectiveSymbolAttribute(MCSA_WeakDefinition);
     case DK_WEAK_REFERENCE:
       return parseDirectiveSymbolAttribute(MCSA_WeakReference);
     case DK_WEAK_DEF_CAN_BE_HIDDEN:
       return parseDirectiveSymbolAttribute(MCSA_WeakDefAutoPrivate);
+    case DK_COLD:
+      return parseDirectiveSymbolAttribute(MCSA_Cold);
     case DK_COMM:
     case DK_COMMON:
       return parseDirectiveComm(/*IsLocal=*/false);
     case DK_LCOMM:
       return parseDirectiveComm(/*IsLocal=*/true);
     case DK_ABORT:
       return parseDirectiveAbort();
     case DK_INCLUDE:
void AsmParser::initializeDirectiveKindMap() {
   DirectiveKindMap[".lazy_reference"] = DK_LAZY_REFERENCE;
   DirectiveKindMap[".no_dead_strip"] = DK_NO_DEAD_STRIP;
   DirectiveKindMap[".symbol_resolver"] = DK_SYMBOL_RESOLVER;
   DirectiveKindMap[".private_extern"] = DK_PRIVATE_EXTERN;
   DirectiveKindMap[".reference"] = DK_REFERENCE;
   DirectiveKindMap[".weak_definition"] = DK_WEAK_DEFINITION;
   DirectiveKindMap[".weak_reference"] = DK_WEAK_REFERENCE;
   DirectiveKindMap[".weak_def_can_be_hidden"] = DK_WEAK_DEF_CAN_BE_HIDDEN;
+  DirectiveKindMap[".cold"] = DK_COLD;
   DirectiveKindMap[".comm"] = DK_COMM;
   DirectiveKindMap[".common"] = DK_COMMON;
   DirectiveKindMap[".lcomm"] = DK_LCOMM;
   DirectiveKindMap[".abort"] = DK_ABORT;
   DirectiveKindMap[".include"] = DK_INCLUDE;
   DirectiveKindMap[".incbin"] = DK_INCBIN;
   DirectiveKindMap[".code16"] = DK_CODE16;
   DirectiveKindMap[".code16gcc"] = DK_CODE16GCC;

llvm/test/MC/AArch64/cold.s

+// RUN: llvm-mc -triple aarch64-apple-ios -o - -filetype=obj %s | \
+// RUN:   llvm-readobj -symbols | FileCheck %s -check-prefix=READOBJ
+
+// READOBJ-LABEL: Name: cold_func
+// READOBJ-NEXT: Type: Section
+// READOBJ-NEXT: Section: __text
+// READOBJ-NEXT: RefType: UndefinedNonLazy (0x0)
+// READOBJ-NEXT: Flags [ (0x400)
+
+  .text
+  .cold cold_func
+cold_func:
+  ret

llvm/test/tools/llvm-nm/AArch64/Inputs/cold-func.ll

+define void @cold_func() cold {
+  ret void
+}

llvm/test/tools/llvm-nm/AArch64/macho-cold.test

+RUN: llc -O0 -mtriple=aarch64-apple-ios %p/Inputs/cold-func.ll -filetype=obj -o %t.aarch64.o
+RUN: llvm-nm -m %t.aarch64.o | FileCheck %s
+
+CHECK: [cold] _cold_func

llvm/tools/llvm-nm/llvm-nm.cpp

if (Filetype == MachO::MH_OBJECT &&
       (NDesc & MachO::N_SYMBOL_RESOLVER) == MachO::N_SYMBOL_RESOLVER)
     outs() << "[symbol resolver] ";
 
   if (Filetype == MachO::MH_OBJECT &&
       ((NType & MachO::N_TYPE) != MachO::N_UNDF) &&
       (NDesc & MachO::N_ALT_ENTRY) == MachO::N_ALT_ENTRY)
     outs() << "[alt entry] ";
 
+  if (Filetype == MachO::MH_OBJECT &&
+      ((NType & MachO::N_TYPE) != MachO::N_UNDF) &&
+      (NDesc & MachO::N_COLD_FUNC) == MachO::N_COLD_FUNC)
+    outs() << "[cold] ";
+
   if ((NDesc & MachO::N_ARM_THUMB_DEF) == MachO::N_ARM_THUMB_DEF)
     outs() << "[Thumb] ";
 
   if ((NType & MachO::N_TYPE) == MachO::N_INDR) {
     outs() << I->Name << " (for ";
     StringRef IndirectName;
     if (MachO) {
       if (I->Sym.getRawDataRefImpl().p) {