[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