[llvm] 30d314a - [MC][WebAssembly] Avoid recalculating indexes in -gsplit-dwarf mode
Sam Clegg via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 6 01:35:19 PST 2021
Author: Sam Clegg
Date: 2021-01-06T01:35:06-08:00
New Revision: 30d314aae10eee1e66aff6515a764ee696a03e8d
URL: https://github.com/llvm/llvm-project/commit/30d314aae10eee1e66aff6515a764ee696a03e8d
DIFF: https://github.com/llvm/llvm-project/commit/30d314aae10eee1e66aff6515a764ee696a03e8d.diff
LOG: [MC][WebAssembly] Avoid recalculating indexes in -gsplit-dwarf mode
Be consistent about asserting before setting WasmIndices. Adding
these assertions revealed that we were duplicating a lot of work
and setting these indexed twice when running in DWO mode.
Differential Revision: https://reviews.llvm.org/D93650
Added:
Modified:
llvm/lib/MC/WasmObjectWriter.cpp
Removed:
################################################################################
diff --git a/llvm/lib/MC/WasmObjectWriter.cpp b/llvm/lib/MC/WasmObjectWriter.cpp
index 683678b70ebc..112c5b3f120b 100644
--- a/llvm/lib/MC/WasmObjectWriter.cpp
+++ b/llvm/lib/MC/WasmObjectWriter.cpp
@@ -1212,6 +1212,7 @@ static bool isInSymtab(const MCSymbolWasm &Sym) {
return true;
}
+
void WasmObjectWriter::prepareImports(
SmallVectorImpl<wasm::WasmImport> &Imports, MCAssembler &Asm,
const MCAsmLayout &Layout) {
@@ -1363,6 +1364,7 @@ uint64_t WasmObjectWriter::writeOneObject(MCAssembler &Asm,
if (Mode != DwoMode::DwoOnly) {
prepareImports(Imports, Asm, Layout);
}
+
// Populate DataSegments and CustomSections, which must be done before
// populating DataLocations.
for (MCSection &Sec : Asm) {
@@ -1417,6 +1419,7 @@ uint64_t WasmObjectWriter::writeOneObject(MCAssembler &Asm,
MCSymbol *Begin = Sec.getBeginSymbol();
if (Begin) {
+ assert(WasmIndices.count(cast<MCSymbolWasm>(Begin)) == 0);
WasmIndices[cast<MCSymbolWasm>(Begin)] = CustomSections.size();
}
@@ -1445,214 +1448,218 @@ uint64_t WasmObjectWriter::writeOneObject(MCAssembler &Asm,
}
}
- // Populate WasmIndices and DataLocations for defined symbols.
- for (const MCSymbol &S : Asm.symbols()) {
- // Ignore unnamed temporary symbols, which aren't ever exported, imported,
- // or used in relocations.
- if (S.isTemporary() && S.getName().empty())
- continue;
+ if (Mode != DwoMode::DwoOnly) {
+ // Populate WasmIndices and DataLocations for defined symbols.
+ for (const MCSymbol &S : Asm.symbols()) {
+ // Ignore unnamed temporary symbols, which aren't ever exported, imported,
+ // or used in relocations.
+ if (S.isTemporary() && S.getName().empty())
+ continue;
- const auto &WS = static_cast<const MCSymbolWasm &>(S);
- LLVM_DEBUG(
- dbgs() << "MCSymbol: " << toString(WS.getType()) << " '" << S << "'"
- << " isDefined=" << S.isDefined() << " isExternal="
- << S.isExternal() << " isTemporary=" << S.isTemporary()
- << " isWeak=" << WS.isWeak() << " isHidden=" << WS.isHidden()
- << " isVariable=" << WS.isVariable() << "\n");
-
- if (WS.isVariable())
- continue;
- if (WS.isComdat() && !WS.isDefined())
- continue;
+ const auto &WS = static_cast<const MCSymbolWasm &>(S);
+ LLVM_DEBUG(
+ dbgs() << "MCSymbol: " << toString(WS.getType()) << " '" << S << "'"
+ << " isDefined=" << S.isDefined() << " isExternal="
+ << S.isExternal() << " isTemporary=" << S.isTemporary()
+ << " isWeak=" << WS.isWeak() << " isHidden=" << WS.isHidden()
+ << " isVariable=" << WS.isVariable() << "\n");
- 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;
- 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});
+ if (WS.isVariable())
+ continue;
+ if (WS.isComdat() && !WS.isDefined())
+ continue;
+
+ 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;
+ 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});
+ }
+
+ if (WS.hasExportName()) {
+ wasm::WasmExport Export;
+ Export.Name = WS.getExportName();
+ Export.Kind = wasm::WASM_EXTERNAL_FUNCTION;
+ Export.Index = Index;
+ Exports.push_back(Export);
+ }
+ } else {
+ // An import; the index was assigned above.
+ Index = WasmIndices.find(&WS)->second;
}
- if (WS.hasExportName()) {
- wasm::WasmExport Export;
- Export.Name = WS.getExportName();
- Export.Kind = wasm::WASM_EXTERNAL_FUNCTION;
- Export.Index = Index;
- Exports.push_back(Export);
+ LLVM_DEBUG(dbgs() << " -> function index: " << Index << "\n");
+
+ } else if (WS.isData()) {
+ if (!isInSymtab(WS))
+ continue;
+
+ if (!WS.isDefined()) {
+ LLVM_DEBUG(dbgs() << " -> segment index: -1"
+ << "\n");
+ continue;
}
- } else {
- // An import; the index was assigned above.
- Index = WasmIndices.find(&WS)->second;
- }
- LLVM_DEBUG(dbgs() << " -> function index: " << Index << "\n");
+ if (!WS.getSize())
+ report_fatal_error("data symbols must have a size set with .size: " +
+ WS.getName());
- } else if (WS.isData()) {
- if (!isInSymtab(WS))
- continue;
+ int64_t Size = 0;
+ if (!WS.getSize()->evaluateAsAbsolute(Size, Layout))
+ report_fatal_error(".size expression must be evaluatable");
- if (!WS.isDefined()) {
- LLVM_DEBUG(dbgs() << " -> segment index: -1"
- << "\n");
- continue;
- }
+ auto &DataSection = static_cast<MCSectionWasm &>(WS.getSection());
+ if (!DataSection.isWasmData())
+ report_fatal_error("data symbols must live in a data section: " +
+ WS.getName());
- if (!WS.getSize())
- report_fatal_error("data symbols must have a size set with .size: " +
- WS.getName());
-
- int64_t Size = 0;
- if (!WS.getSize()->evaluateAsAbsolute(Size, Layout))
- report_fatal_error(".size expression must be evaluatable");
-
- auto &DataSection = static_cast<MCSectionWasm &>(WS.getSection());
- if (!DataSection.isWasmData())
- report_fatal_error("data symbols must live in a data section: " +
- WS.getName());
-
- // For each data symbol, export it in the symtab as a reference to the
- // corresponding Wasm data segment.
- wasm::WasmDataReference Ref = wasm::WasmDataReference{
- DataSection.getSegmentIndex(), Layout.getSymbolOffset(WS),
- static_cast<uint64_t>(Size)};
- DataLocations[&WS] = Ref;
- LLVM_DEBUG(dbgs() << " -> segment index: " << Ref.Segment << "\n");
-
- } else if (WS.isGlobal()) {
- // A "true" Wasm global (currently just __stack_pointer)
- if (WS.isDefined()) {
- assert(WasmIndices.count(&WS) == 0);
- wasm::WasmGlobal Global;
- Global.Type = WS.getGlobalType();
- Global.Index = NumGlobalImports + Globals.size();
- switch (Global.Type.Type) {
- case wasm::WASM_TYPE_I32:
- Global.InitExpr.Opcode = wasm::WASM_OPCODE_I32_CONST;
- break;
- case wasm::WASM_TYPE_I64:
- Global.InitExpr.Opcode = wasm::WASM_OPCODE_I64_CONST;
- break;
- case wasm::WASM_TYPE_F32:
- Global.InitExpr.Opcode = wasm::WASM_OPCODE_F32_CONST;
- break;
- case wasm::WASM_TYPE_F64:
- Global.InitExpr.Opcode = wasm::WASM_OPCODE_F64_CONST;
- break;
- case wasm::WASM_TYPE_EXTERNREF:
- Global.InitExpr.Opcode = wasm::WASM_OPCODE_REF_NULL;
- break;
- default:
- llvm_unreachable("unexpected type");
+ // For each data symbol, export it in the symtab as a reference to the
+ // corresponding Wasm data segment.
+ wasm::WasmDataReference Ref = wasm::WasmDataReference{
+ DataSection.getSegmentIndex(), Layout.getSymbolOffset(WS),
+ static_cast<uint64_t>(Size)};
+ assert(DataLocations.count(&WS) == 0);
+ DataLocations[&WS] = Ref;
+ LLVM_DEBUG(dbgs() << " -> segment index: " << Ref.Segment << "\n");
+
+ } else if (WS.isGlobal()) {
+ // A "true" Wasm global (currently just __stack_pointer)
+ if (WS.isDefined()) {
+ wasm::WasmGlobal Global;
+ Global.Type = WS.getGlobalType();
+ Global.Index = NumGlobalImports + Globals.size();
+ switch (Global.Type.Type) {
+ case wasm::WASM_TYPE_I32:
+ Global.InitExpr.Opcode = wasm::WASM_OPCODE_I32_CONST;
+ break;
+ case wasm::WASM_TYPE_I64:
+ Global.InitExpr.Opcode = wasm::WASM_OPCODE_I64_CONST;
+ break;
+ case wasm::WASM_TYPE_F32:
+ Global.InitExpr.Opcode = wasm::WASM_OPCODE_F32_CONST;
+ break;
+ case wasm::WASM_TYPE_F64:
+ Global.InitExpr.Opcode = wasm::WASM_OPCODE_F64_CONST;
+ break;
+ case wasm::WASM_TYPE_EXTERNREF:
+ Global.InitExpr.Opcode = wasm::WASM_OPCODE_REF_NULL;
+ break;
+ default:
+ llvm_unreachable("unexpected type");
+ }
+ assert(WasmIndices.count(&WS) == 0);
+ WasmIndices[&WS] = Global.Index;
+ Globals.push_back(Global);
+ } else {
+ // An import; the index was assigned above
+ LLVM_DEBUG(dbgs() << " -> global index: "
+ << WasmIndices.find(&WS)->second << "\n");
}
- WasmIndices[&WS] = Global.Index;
- Globals.push_back(Global);
- } else {
- // An import; the index was assigned above
- LLVM_DEBUG(dbgs() << " -> global index: "
+ } else if (WS.isTable()) {
+ if (WS.isDefined()) {
+ wasm::WasmTable Table;
+ Table.Index = NumTableImports + Tables.size();
+ Table.Type.ElemType = static_cast<uint8_t>(WS.getTableType());
+ // FIXME: Work on custom limits is ongoing
+ Table.Type.Limits = {wasm::WASM_LIMITS_FLAG_NONE, 0, 0};
+ assert(WasmIndices.count(&WS) == 0);
+ WasmIndices[&WS] = Table.Index;
+ Tables.push_back(Table);
+ }
+ LLVM_DEBUG(dbgs() << " -> table index: "
<< WasmIndices.find(&WS)->second << "\n");
- }
- } else if (WS.isTable()) {
- if (WS.isDefined()) {
- assert(WasmIndices.count(&WS) == 0);
- wasm::WasmTable Table;
- Table.Index = NumTableImports + Tables.size();
- Table.Type.ElemType = static_cast<uint8_t>(WS.getTableType());
- // FIXME: Work on custom limits is ongoing
- Table.Type.Limits = {wasm::WASM_LIMITS_FLAG_NONE, 0, 0};
-
- WasmIndices[&WS] = Table.Index;
- Tables.push_back(Table);
- }
- LLVM_DEBUG(dbgs() << " -> table index: " << WasmIndices.find(&WS)->second
- << "\n");
- } else if (WS.isEvent()) {
- // C++ exception symbol (__cpp_exception)
- unsigned Index;
- if (WS.isDefined()) {
- assert(WasmIndices.count(&WS) == 0);
- Index = NumEventImports + Events.size();
- wasm::WasmEventType Event;
- Event.SigIndex = getEventType(WS);
- Event.Attribute = wasm::WASM_EVENT_ATTRIBUTE_EXCEPTION;
- WasmIndices[&WS] = Index;
- Events.push_back(Event);
+ } else if (WS.isEvent()) {
+ // C++ exception symbol (__cpp_exception)
+ unsigned Index;
+ if (WS.isDefined()) {
+ Index = NumEventImports + Events.size();
+ wasm::WasmEventType Event;
+ Event.SigIndex = getEventType(WS);
+ Event.Attribute = wasm::WASM_EVENT_ATTRIBUTE_EXCEPTION;
+ assert(WasmIndices.count(&WS) == 0);
+ WasmIndices[&WS] = Index;
+ Events.push_back(Event);
+ } else {
+ // An import; the index was assigned above.
+ assert(WasmIndices.count(&WS) > 0);
+ }
+ LLVM_DEBUG(dbgs() << " -> event index: "
+ << WasmIndices.find(&WS)->second << "\n");
+
} else {
- // An import; the index was assigned above.
- assert(WasmIndices.count(&WS) > 0);
+ assert(WS.isSection());
}
- LLVM_DEBUG(dbgs() << " -> event index: " << WasmIndices.find(&WS)->second
- << "\n");
-
- } else {
- assert(WS.isSection());
}
- }
- // Populate WasmIndices and DataLocations for aliased symbols. We need to
- // process these in a separate pass because we need to have processed the
- // target of the alias before the alias itself and the symbols are not
- // necessarily ordered in this way.
- for (const MCSymbol &S : Asm.symbols()) {
- if (!S.isVariable())
- continue;
+ // Populate WasmIndices and DataLocations for aliased symbols. We need to
+ // process these in a separate pass because we need to have processed the
+ // target of the alias before the alias itself and the symbols are not
+ // necessarily ordered in this way.
+ for (const MCSymbol &S : Asm.symbols()) {
+ if (!S.isVariable())
+ continue;
+
+ assert(S.isDefined());
- assert(S.isDefined());
+ const auto *BS = Layout.getBaseSymbol(S);
+ if (!BS)
+ report_fatal_error(Twine(S.getName()) +
+ ": absolute addressing not supported!");
+ const MCSymbolWasm *Base = cast<MCSymbolWasm>(BS);
- const auto *BS = Layout.getBaseSymbol(S);
- if (!BS)
- report_fatal_error(Twine(S.getName()) +
- ": absolute addressing not supported!");
- const MCSymbolWasm *Base = cast<MCSymbolWasm>(BS);
+ // Find the target symbol of this weak alias and export that index
+ const auto &WS = static_cast<const MCSymbolWasm &>(S);
+ LLVM_DEBUG(dbgs() << WS.getName() << ": weak alias of '" << *Base
+ << "'\n");
- // Find the target symbol of this weak alias and export that index
- const auto &WS = static_cast<const MCSymbolWasm &>(S);
- LLVM_DEBUG(dbgs() << WS.getName() << ": weak alias of '" << *Base << "'\n");
-
- if (Base->isFunction()) {
- assert(WasmIndices.count(Base) > 0);
- uint32_t WasmIndex = WasmIndices.find(Base)->second;
- assert(WasmIndices.count(&WS) == 0);
- WasmIndices[&WS] = WasmIndex;
- LLVM_DEBUG(dbgs() << " -> index:" << WasmIndex << "\n");
- } else if (Base->isData()) {
- auto &DataSection = static_cast<MCSectionWasm &>(WS.getSection());
- uint64_t Offset = Layout.getSymbolOffset(S);
- int64_t Size = 0;
- // For data symbol alias we use the size of the base symbol as the
- // size of the alias. When an offset from the base is involved this
- // can result in a offset + size goes past the end of the data section
- // which out object format doesn't support. So we must clamp it.
- if (!Base->getSize()->evaluateAsAbsolute(Size, Layout))
- report_fatal_error(".size expression must be evaluatable");
- const WasmDataSegment &Segment =
- DataSegments[DataSection.getSegmentIndex()];
- Size =
- std::min(static_cast<uint64_t>(Size), Segment.Data.size() - Offset);
- wasm::WasmDataReference Ref = wasm::WasmDataReference{
- DataSection.getSegmentIndex(),
- static_cast<uint32_t>(Layout.getSymbolOffset(S)),
- static_cast<uint32_t>(Size)};
- DataLocations[&WS] = Ref;
- LLVM_DEBUG(dbgs() << " -> index:" << Ref.Segment << "\n");
- } else {
- report_fatal_error("don't yet support global/event aliases");
+ if (Base->isFunction()) {
+ assert(WasmIndices.count(Base) > 0);
+ uint32_t WasmIndex = WasmIndices.find(Base)->second;
+ assert(WasmIndices.count(&WS) == 0);
+ WasmIndices[&WS] = WasmIndex;
+ LLVM_DEBUG(dbgs() << " -> index:" << WasmIndex << "\n");
+ } else if (Base->isData()) {
+ auto &DataSection = static_cast<MCSectionWasm &>(WS.getSection());
+ uint64_t Offset = Layout.getSymbolOffset(S);
+ int64_t Size = 0;
+ // For data symbol alias we use the size of the base symbol as the
+ // size of the alias. When an offset from the base is involved this
+ // can result in a offset + size goes past the end of the data section
+ // which out object format doesn't support. So we must clamp it.
+ if (!Base->getSize()->evaluateAsAbsolute(Size, Layout))
+ report_fatal_error(".size expression must be evaluatable");
+ const WasmDataSegment &Segment =
+ DataSegments[DataSection.getSegmentIndex()];
+ Size =
+ std::min(static_cast<uint64_t>(Size), Segment.Data.size() - Offset);
+ wasm::WasmDataReference Ref = wasm::WasmDataReference{
+ DataSection.getSegmentIndex(),
+ static_cast<uint32_t>(Layout.getSymbolOffset(S)),
+ static_cast<uint32_t>(Size)};
+ DataLocations[&WS] = Ref;
+ LLVM_DEBUG(dbgs() << " -> index:" << Ref.Segment << "\n");
+ } else {
+ report_fatal_error("don't yet support global/event aliases");
+ }
}
}
More information about the llvm-commits
mailing list