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"); + } } } _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits