[WebAssembly][MC] Avoid the need for .size directives for functions

[sbc100]

Summary

Warn if .size is specified for a function symbol. The size of a
function symbol is determined solely by its content.

I noticed this simplification was possible while debugging #57427, but
this change doesn't fix that specific issue.

[sbc100]

[dschuff]

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

1702

This isn't for .size, but for the func_end symbol. It looks like (line 1727 below) the func end is stored in a list of MBB section ranges, and MBBSectionRanges is used for debug info generation. Does removing this symbol have an effect on debug info?

1710

It does seem odd that there's a target hook for whether there's a .type/.size directive but we can't use it. Is that just because we want .type (and .size for objects?) but not for functions?

[sbc100]

• add comment

[sbc100]

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

1702

Right, but the func_end only has two possible uses.

1. needFuncLabelsForEHOrDebugInfo
2. For calculating the .size

If neither of these is true there is no possible usage of the symbol. So we don't want to both creating this symbol at all except in these two cases.

1710

I added a comment and a new local variable to explain the situation.

[sbc100]

• more

[dschuff]

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

1702

oh haha right I forgot about the other half of the conditional :D

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

for (const auto &BB : F) {
     OutStreamer->AddComment("Address of block that was removed by CodeGen");
     OutStreamer->emitLabel(Sym);
   }
 
   // Emit target-specific gunk after the function body.
   emitFunctionBodyEnd();
 
   if (needFuncLabelsForEHOrDebugInfo(*MF) ||
-      MAI->hasDotTypeDotSizeDirective()) {
+      (MAI->hasDotTypeDotSizeDirective() && !TT.isWasm())) {
     // Create a symbol for the end of function.
     CurrentFnEnd = createTempSymbol("func_end");
     OutStreamer->emitLabel(CurrentFnEnd);
   }
 
   // If the target wants a .size directive for the size of the function, emit
   // it.
-  if (MAI->hasDotTypeDotSizeDirective()) {
+  if (MAI->hasDotTypeDotSizeDirective() && !TT.isWasm()) {
     // We can get the size as difference between the function label and the
     // temp label.
     const MCExpr *SizeExp = MCBinaryExpr::createSub(
         MCSymbolRefExpr::create(CurrentFnEnd, OutContext),
         MCSymbolRefExpr::create(CurrentFnSymForSize, OutContext), OutContext);
     OutStreamer->emitELFSize(CurrentFnSym, SizeExp);
     if (CurrentFnBeginLocal)
       OutStreamer->emitELFSize(CurrentFnBeginLocal, SizeExp);

llvm/lib/MC/MCParser/WasmAsmParser.cpp

bool parseSectionDirective(StringRef, SMLoc loc) {
     }
 
     getStreamer().switchSection(WS);
     return false;
   }
 
   // TODO: This function is almost the same as ELFAsmParser::ParseDirectiveSize
   // so maybe could be shared somehow.
-  bool parseDirectiveSize(StringRef, SMLoc) {
+  bool parseDirectiveSize(StringRef, SMLoc Loc) {
     StringRef Name;
     if (Parser->parseIdentifier(Name))
       return TokError("expected identifier in directive");
     auto Sym = getContext().getOrCreateSymbol(Name);
     if (expect(AsmToken::Comma, ","))
       return true;
     const MCExpr *Expr;
     if (Parser->parseExpression(Expr))
       return true;
     if (expect(AsmToken::EndOfStatement, "eol"))
       return true;
-    // This is done automatically by the assembler for functions currently,
-    // so this is only currently needed for data sections:
+    auto WasmSym = cast<MCSymbolWasm>(Sym);
+    if (WasmSym->isFunction()) {
+      // Ignore .size directives for function symbols.  They get their size
+      // set automatically based on their content.
+      Warning(Loc, ".size directive ignored for function symbols");
+    } else {
       getStreamer().emitELFSize(Sym, Expr);
+    }
     return false;
   }
 
   bool parseDirectiveType(StringRef, SMLoc) {
     // This could be the start of a function, check if followed by
     // "label,@function"
     if (!Lexer->is(AsmToken::Identifier))
       return error("Expected label after .type directive, got: ",

llvm/lib/MC/WasmObjectWriter.cpp

struct WasmDataSegment {
   uint32_t Alignment;
   uint32_t LinkingFlags;
   SmallVector<char, 4> Data;
 };
 
 // A wasm function to be written into the function section.
 struct WasmFunction {
   uint32_t SigIndex;
-  const MCSymbolWasm *Sym;
+  MCSection *Section;
 };
 
 // A wasm global to be written into the global section.
 struct WasmGlobal {
   wasm::WasmGlobalType Type;
   uint64_t InitialValue;
 };
 
if (Functions.empty())
     return 0;
 
   SectionBookkeeping Section;
   startSection(Section, wasm::WASM_SEC_CODE);
 
   encodeULEB128(Functions.size(), W->OS);
 
   for (const WasmFunction &Func : Functions) {
-    auto &FuncSection = static_cast<MCSectionWasm &>(Func.Sym->getSection());
-
-    int64_t Size = 0;
-    if (!Func.Sym->getSize()->evaluateAsAbsolute(Size, Layout))
-      report_fatal_error(".size expression must be evaluatable");
+    auto *FuncSection = static_cast<MCSectionWasm *>(Func.Section);
 
+    int64_t Size = Layout.getSectionAddressSize(FuncSection);
     encodeULEB128(Size, W->OS);
-    FuncSection.setSectionOffset(W->OS.tell() - Section.ContentsOffset);
-    Asm.writeSectionData(W->OS, &FuncSection, Layout);
+    FuncSection->setSectionOffset(W->OS.tell() - Section.ContentsOffset);
+    Asm.writeSectionData(W->OS, FuncSection, Layout);
   }
 
   // Apply fixups.
   applyRelocations(CodeRelocations, Section.ContentsOffset, Layout);
 
   endSection(Section);
   return Section.Index;
 }
for (const MCSymbol &S : Asm.symbols()) {
 
       if (WS.isFunction()) {
         unsigned Index;
         if (WS.isDefined()) {
           if (WS.getOffset() != 0)
             report_fatal_error(
                 "function sections must contain one function each");
 
-          if (WS.getSize() == nullptr)
-            report_fatal_error(
-                "function symbols must have a size set with .size");
-
           // A definition. Write out the function body.
           Index = NumFunctionImports + Functions.size();
           WasmFunction Func;
           Func.SigIndex = getFunctionType(WS);
-          Func.Sym = &WS;
+          Func.Section = &WS.getSection();
           assert(WasmIndices.count(&WS) == 0);
           WasmIndices[&WS] = Index;
           Functions.push_back(Func);
 
           auto &Section = static_cast<MCSectionWasm &>(WS.getSection());
           if (const MCSymbolWasm *C = Section.getGroup()) {
             Comdats[C->getName()].emplace_back(
                 WasmComdatEntry{wasm::WASM_COMDAT_FUNCTION, Index});

llvm/test/MC/WebAssembly/function-size-warning.s

+# RUN: llvm-mc -triple=wasm32-unknown-unknown -filetype=obj %s -o %t.o 2>&1 | FileCheck %s
+# RUN: llvm-objdump -t %t.o
+
+foo:
+  .functype foo () -> ()
+  i32.const 1
+  drop
+  end_function
+
+# .size directives for functions are no longer required and will
+# be ignored but we continue to allow them to support legacy
+# assembly files.
+.size foo, 0
+
+# CHECK: warning: .size directive ignored for function symbols

llvm/test/MC/WebAssembly/stack-ptr-mclower.ll

 
 ; CHECK-NEXT:         .section        .text.bar,"",@
 ; CHECK-NEXT:         .hidden bar
 ; CHECK-NEXT:         .globl  bar
 ; CHECK-NEXT:         .type   bar,@function
 ; CHECK-NEXT: bar:
 ; CHECK-NEXT:         .functype       bar () -> ()
 ; CHECK-NEXT:         end_function
-; CHECK-NEXT: .Lfunc_end0:
-; CHECK-NEXT:         .size   bar, .Lfunc_end0-bar
 
 ; CHECK:              .section        .text.foo,"",@
 ; CHECK-NEXT:         .hidden foo
 ; CHECK-NEXT:         .globl  foo
 ; CHECK-NEXT:         .type   foo,@function
 ; CHECK-NEXT: foo:
 ; CHECK-NEXT:         .functype       foo () -> ()
 ; CHECK-NEXT:         global.get      __stack_pointer
 ; CHECK-NEXT:         [[PTR]].const       16
 ; CHECK-NEXT:         [[PTR]].sub
 ; CHECK-NEXT:         drop
 ; CHECK-NEXT:         end_function
-; CHECK-NEXT: .Lfunc_end1:
-; CHECK-NEXT:         .size   foo, .Lfunc_end1-foo