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

Sam Clegg via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 31 14:32:52 PDT 2022


Author: Sam Clegg
Date: 2022-08-31T14:28:56-07:00
New Revision: c5c4ba37b19e14f3f4c9c1bf950d46e90602c084

URL: https://github.com/llvm/llvm-project/commit/c5c4ba37b19e14f3f4c9c1bf950d46e90602c084
DIFF: https://github.com/llvm/llvm-project/commit/c5c4ba37b19e14f3f4c9c1bf950d46e90602c084.diff

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

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.

Differential Revision: https://reviews.llvm.org/D132929

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

Modified: 
    llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
    llvm/lib/MC/MCParser/WasmAsmParser.cpp
    llvm/lib/MC/WasmObjectWriter.cpp
    llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
    llvm/test/MC/WebAssembly/stack-ptr-mclower.ll
    llvm/test/MC/WebAssembly/tables.s
    llvm/test/MC/WebAssembly/wasm64.s

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index ea9399c07eb08..c77c4ebd7f922 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -1698,8 +1698,11 @@ void AsmPrinter::emitFunctionBody() {
   // 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);
@@ -1707,7 +1710,7 @@ void AsmPrinter::emitFunctionBody() {
 
   // 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 
diff erence between the function label and the
     // temp label.
     const MCExpr *SizeExp = MCBinaryExpr::createSub(

diff  --git a/llvm/lib/MC/MCParser/WasmAsmParser.cpp b/llvm/lib/MC/MCParser/WasmAsmParser.cpp
index a84d00d82b769..7f9c4050d3837 100644
--- a/llvm/lib/MC/MCParser/WasmAsmParser.cpp
+++ b/llvm/lib/MC/MCParser/WasmAsmParser.cpp
@@ -207,7 +207,7 @@ class WasmAsmParser : public MCAsmParserExtension {
 
   // 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");
@@ -219,9 +219,14 @@ class WasmAsmParser : public MCAsmParserExtension {
       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:
-    getStreamer().emitELFSize(Sym, Expr);
+    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;
   }
 

diff  --git a/llvm/lib/MC/WasmObjectWriter.cpp b/llvm/lib/MC/WasmObjectWriter.cpp
index 7cc11d24f286c..04049292ebff5 100644
--- a/llvm/lib/MC/WasmObjectWriter.cpp
+++ b/llvm/lib/MC/WasmObjectWriter.cpp
@@ -72,7 +72,7 @@ struct WasmDataSegment {
 // 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.
@@ -1058,15 +1058,12 @@ uint32_t WasmObjectWriter::writeCodeSection(const MCAssembler &Asm,
   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.
@@ -1591,15 +1588,11 @@ uint64_t WasmObjectWriter::writeOneObject(MCAssembler &Asm,
             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);

diff  --git a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
index 7bafa53af2af3..1323d7bd0b9eb 100644
--- a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
+++ b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
@@ -1106,17 +1106,6 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
       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(); }

diff  --git a/llvm/test/MC/WebAssembly/function-size-warning.s b/llvm/test/MC/WebAssembly/function-size-warning.s
new file mode 100644
index 0000000000000..627002dd35781
--- /dev/null
+++ b/llvm/test/MC/WebAssembly/function-size-warning.s
@@ -0,0 +1,15 @@
+# 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

diff  --git a/llvm/test/MC/WebAssembly/stack-ptr-mclower.ll b/llvm/test/MC/WebAssembly/stack-ptr-mclower.ll
index 1ebcbe3d81cf7..f545e9c7cdb27 100644
--- a/llvm/test/MC/WebAssembly/stack-ptr-mclower.ll
+++ b/llvm/test/MC/WebAssembly/stack-ptr-mclower.ll
@@ -28,8 +28,6 @@ entry:
 ; 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
@@ -42,5 +40,3 @@ entry:
 ; CHECK-NEXT:         [[PTR]].sub
 ; CHECK-NEXT:         drop
 ; CHECK-NEXT:         end_function
-; CHECK-NEXT: .Lfunc_end1:
-; CHECK-NEXT:         .size   foo, .Lfunc_end1-foo

diff  --git a/llvm/test/MC/WebAssembly/tables.s b/llvm/test/MC/WebAssembly/tables.s
index a682a738445b7..8239432eed88d 100644
--- a/llvm/test/MC/WebAssembly/tables.s
+++ b/llvm/test/MC/WebAssembly/tables.s
@@ -29,8 +29,6 @@ table2:
 #      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
@@ -48,8 +46,6 @@ copy_tables:
 # 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
@@ -64,8 +60,6 @@ table_get:
 # 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
@@ -84,8 +78,6 @@ table_set:
 # 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
@@ -106,8 +98,6 @@ table_grow:
 # 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

diff  --git a/llvm/test/MC/WebAssembly/wasm64.s b/llvm/test/MC/WebAssembly/wasm64.s
index c41f4f0c96009..0c939b47db4f2 100644
--- a/llvm/test/MC/WebAssembly/wasm64.s
+++ b/llvm/test/MC/WebAssembly/wasm64.s
@@ -119,8 +119,6 @@ test:
 
 
 # CHECK:              end_function
-# CHECK-NEXT: .Ltmp0:
-# CHECK-NEXT:         .size   test, .Ltmp0-test
 
 # CHECK:              .section        .rodata..L.str,"",@
 # CHECK-NEXT:         .hidden .L.str


        


More information about the llvm-commits mailing list