[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

1701–1705

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?

1713

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

1701–1705

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.

1713

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

[sbc100]

• more

[dschuff]

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

1701–1705

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

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

if (Sym->isDefined())
       continue;
     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()) {
+  // Even though wasm supports .type and .size in general, function symbols
+  // are automatically sized.
+  bool EmitFunctionSize = MAI->hasDotTypeDotSizeDirective() && !TT.isWasm();
+
+  if (needFuncLabelsForEHOrDebugInfo(*MF) || EmitFunctionSize) {
     // 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 (EmitFunctionSize) {
     // 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/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp

if (getContext().getGenDwarfForAssembly())
       getContext().addGenDwarfSection(WS);
   }
 
   void onEndOfFunction(SMLoc ErrorLoc) {
     if (!SkipTypeCheck)
       TC.endOfFunction(ErrorLoc);
     // Reset the type checker state.
     TC.Clear();
-
-    // Automatically output a .size directive, so it becomes optional for the
-    // user.
-    if (!LastFunctionLabel) return;
-    auto TempSym = getContext().createLinkerPrivateTempSymbol();
-    getStreamer().emitLabel(TempSym);
-    auto Start = MCSymbolRefExpr::create(LastFunctionLabel, getContext());
-    auto End = MCSymbolRefExpr::create(TempSym, getContext());
-    auto Expr =
-        MCBinaryExpr::create(MCBinaryExpr::Sub, End, Start, getContext());
-    getStreamer().emitELFSize(LastFunctionLabel, Expr);
   }
 
   void onEndOfFile() override { ensureEmptyNestingStack(); }
 };
 } // end anonymous namespace
 
 // Force static initialization.
 extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeWebAssemblyAsmParser() {

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

llvm/test/MC/WebAssembly/tables.s

 
 #      CHECK: copy_tables:
 # CHECK-NEXT:         .functype copy_tables (i32, i32) -> ()
 # CHECK-NEXT: local.get 0
 # CHECK-NEXT: local.get 1
 #      CHECK: table.size  table1
 #      CHECK: table.copy  table1, table2
 # CHECK-NEXT: end_function
-# CHECK-NEXT:.Ltmp0:
-# CHECK-NEXT: .size copy_tables, .Ltmp0-copy_tables
 copy_tables:
     .functype copy_tables (i32, i32) -> ()
     local.get 0
     local.get 1
 
     # ENC: table.size table1                  # encoding: [0xfc,0x10,0x80'A',0x80'A',0x80'A',0x80'A',A]
     table.size table1
 
     # ENC: table.copy table1, table2          # encoding: [0xfc,0x0e,0x80'A',0x80'A',0x80'A',0x80'A',A,0x80'B',0x80'B',0x80'B',0x80'B',B]
     table.copy table1, table2
     end_function
 
 #      CHECK: table_get:
 # CHECK-NEXT: .functype table_get (i32) -> (externref)
 # CHECK-NEXT: local.get 0
 #      CHECK: table.get foo
 # CHECK-NEXT: end_function
-# CHECK-NEXT: .Ltmp1:
-# CHECK-NEXT: .size table_get, .Ltmp1-table_get
 table_get:
     .functype table_get (i32) -> (externref)
     local.get 0
 
     # ENC: table.get  foo                     # encoding: [0x25,0x80'A',0x80'A',0x80'A',0x80'A',A]
     table.get foo
     end_function
 
 #      CHECK: table_set:
 # CHECK-NEXT: .functype table_set (i32, externref) -> ()
 # CHECK-NEXT: local.get 0
 # CHECK-NEXT: local.get 1
 #      CHECK: table.set foo
 # CHECK-NEXT: end_function
-# CHECK-NEXT: .Ltmp2:
-# CHECK-NEXT: .size table_set, .Ltmp2-table_set
 table_set:
     .functype table_set (i32, externref) -> ()
     local.get 0
     local.get 1
 
     # ENC: table.set  foo                     # encoding: [0x26,0x80'A',0x80'A',0x80'A',0x80'A',A]
     table.set foo
     end_function
 
 #      CHECK: table_grow:
 # CHECK-NEXT: .functype table_grow (i32) -> (i32)
 # CHECK-NEXT: i32.const 0
 # CHECK-NEXT: table.get foo
 # CHECK-NEXT: local.get 0
 #      CHECK: table.grow  foo
 # CHECK-NEXT: local.get 0
 # CHECK-NEXT: i32.add
 # CHECK-NEXT: end_function
-# CHECK-NEXT: .Ltmp3:
-# CHECK-NEXT: .size table_grow, .Ltmp3-table_grow
 table_grow:
     .functype table_grow (i32) -> (i32)
     i32.const 0
     table.get foo
     local.get 0
 
     # ENC: table.grow foo                     # encoding: [0xfc,0x0f,0x80'A',0x80'A',0x80'A',0x80'A',A]
     table.grow foo
     local.get 0
     i32.add
     end_function
 
 #      CHECK: table_fill:
 # CHECK-NEXT: .functype table_fill (i32, i32) -> ()
 # CHECK-NEXT: local.get 0
 # CHECK-NEXT: i32.const 0
 # CHECK-NEXT: table.get table1
 # CHECK-NEXT: local.get 1
 #      CHECK: table.fill  table1
 # CHECK-NEXT: end_function
-# CHECK-NEXT: .Ltmp4:
-# CHECK-NEXT: .size table_fill, .Ltmp4-table_fill
 table_fill:
     .functype table_fill (i32, i32) -> ()
     local.get 0
     i32.const 0
     table.get table1
     local.get 1
 
     # ENC: table.fill table1                  # encoding: [0xfc,0x11,0x80'A',0x80'A',0x80'A',0x80'A',A]

llvm/test/MC/WebAssembly/wasm64.s

 # CHECK-NEXT:         f32.store       0
 
 # CHECK:              i64.const       0
 # CHECK-NEXT:         f32.const       0x0p0
 # CHECK-NEXT:         f32.store       .L.str
 
 
 # CHECK:              end_function
-# CHECK-NEXT: .Ltmp0:
-# CHECK-NEXT:         .size   test, .Ltmp0-test
 
 # CHECK:              .section        .rodata..L.str,"",@
 # CHECK-NEXT:         .hidden .L.str
 # CHECK-NEXT: .L.str:
 # CHECK-NEXT:         .asciz  "Hello, World!!!"
 # CHECK-NEXT:         .int64      .L.str
 # CHECK-NEXT:         .size       .L.str, 24