Page MenuHomePhabricator
Differential D64537 Diff 209274 lld/wasm/Writer.cpp

Changeset View

Standalone View

View Options

lld/wasm/Writer.cpp

Show First 20 Lines • Show All 51 Lines • ▼ Show 20 Linespublic:
void run(); void run();
private: private:
void openFile(); void openFile();
void createInitMemoryFunction(); void createInitMemoryFunction();
void createApplyRelocationsFunction(); void createApplyRelocationsFunction();
void createCallCtorsFunction(); void createCallCtorsFunction();
void createInitTLSFunction();
void assignIndexes(); void assignIndexes();
void populateSymtab(); void populateSymtab();
void populateProducers(); void populateProducers();
void populateTargetFeatures(); void populateTargetFeatures();
void calculateInitFunctions(); void calculateInitFunctions();
void calculateImports(); void calculateImports();
void calculateExports(); void calculateExports();
▲ Show 20 Lines • Show All 169 Lines • ▼ Show 20 Linesvoid Writer::layoutMemory() {
Out.DylinkSec->MemAlign = 0; Out.DylinkSec->MemAlign = 0;
for (OutputSegment *Seg : Segments) { for (OutputSegment *Seg : Segments) {
Out.DylinkSec->MemAlign = std::max(Out.DylinkSec->MemAlign, Seg->Alignment); Out.DylinkSec->MemAlign = std::max(Out.DylinkSec->MemAlign, Seg->Alignment);
MemoryPtr = alignTo(MemoryPtr, 1ULL << Seg->Alignment); MemoryPtr = alignTo(MemoryPtr, 1ULL << Seg->Alignment);
Seg->StartVA = MemoryPtr; Seg->StartVA = MemoryPtr;
log(formatv("mem: {0,-15} offset={1,-8} size={2,-8} align={3}", Seg->Name, log(formatv("mem: {0,-15} offset={1,-8} size={2,-8} align={3}", Seg->Name,
MemoryPtr, Seg->Size, Seg->Alignment)); MemoryPtr, Seg->Size, Seg->Alignment));
MemoryPtr += Seg->Size; MemoryPtr += Seg->Size;
if (WasmSym::TLSSize && Seg->Name == ".tdata") {
auto *TLSSize = cast<DefinedGlobal>(WasmSym::TLSSize);
TLSSize->Global->Global.InitExpr.Value.Int32 = Seg->Size;
}
tlivelyUnsubmitted
Done
ReplyInline Actions

What happens when there are multiple TLS sections and --no-merge-data-segments is used? I assume their sizes should be added together?

tlively: What happens when there are multiple TLS sections and `--no-merge-data-segments` is used? I…
quantumAuthorUnsubmitted
Done
ReplyInline Actions

I'll merge the TLS segments anyways despite --no-merge-data-segments.

quantum: I'll merge the TLS segments anyways despite `--no-merge-data-segments`.
} }
// TODO: Add .bss space here. // TODO: Add .bss space here.
if (WasmSym::DataEnd) if (WasmSym::DataEnd)
WasmSym::DataEnd->setVirtualAddress(MemoryPtr); WasmSym::DataEnd->setVirtualAddress(MemoryPtr);
log("mem: static data = " + Twine(MemoryPtr - DataStart)); log("mem: static data = " + Twine(MemoryPtr - DataStart));
▲ Show 20 Lines • Show All 95 Lines • ▼ Show 20 Lines for (OutputSection *S : OutputSections) {
FileSize += S->getSize(); FileSize += S->getSize();
} }
} }
void Writer::populateTargetFeatures() { void Writer::populateTargetFeatures() {
StringMap<std::string> Used; StringMap<std::string> Used;
StringMap<std::string> Required; StringMap<std::string> Required;
StringMap<std::string> Disallowed; StringMap<std::string> Disallowed;
bool TLSUsed = false;
// Only infer used features if user did not specify features // Only infer used features if user did not specify features
bool InferFeatures = !Config->Features.hasValue(); bool InferFeatures = !Config->Features.hasValue();
if (!InferFeatures) { if (!InferFeatures) {
for (auto &Feature : Config->Features.getValue()) for (auto &Feature : Config->Features.getValue())
Out.TargetFeaturesSec->Features.insert(Feature); Out.TargetFeaturesSec->Features.insert(Feature);
// No need to read or check features // No need to read or check features
Show All 16 Lines for (auto &Feature : File->getWasmObj()->getTargetFeatures()) {
case WASM_FEATURE_PREFIX_DISALLOWED: case WASM_FEATURE_PREFIX_DISALLOWED:
Disallowed.insert({Feature.Name, FileName}); Disallowed.insert({Feature.Name, FileName});
break; break;
default: default:
error("Unrecognized feature policy prefix " + error("Unrecognized feature policy prefix " +
std::to_string(Feature.Prefix)); std::to_string(Feature.Prefix));
} }
} }
for (InputSegment *Segment : File->Segments) {
if (!Segment->Live)
continue;
StringRef Name = Segment->getName();
if (Name.startswith(".tdata.") || Name.startswith(".tbss."))
sbc100Unsubmitted
Done
ReplyInline Actions

Right now we always compiler with -fdata-sections, but if we didn't then object files might have just one ".tdata" sections. Perhaps drop the final "."?

sbc100: Right now we always compiler with -fdata-sections, but if we didn't then object files might…
TLSUsed = true;
}
} }
if (InferFeatures) if (InferFeatures)
Out.TargetFeaturesSec->Features.insert(Used.keys().begin(), Out.TargetFeaturesSec->Features.insert(Used.keys().begin(),
Used.keys().end()); Used.keys().end());
if (Out.TargetFeaturesSec->Features.count("atomics") && if (Out.TargetFeaturesSec->Features.count("atomics") &&
!Config->SharedMemory) { !Config->SharedMemory) {
Show All 10 Linesvoid Writer::populateTargetFeatures() {
if (Disallowed.count("atomics") && Config->SharedMemory) if (Disallowed.count("atomics") && Config->SharedMemory)
error("'atomics' feature is disallowed by " + Disallowed["atomics"] + error("'atomics' feature is disallowed by " + Disallowed["atomics"] +
", so --shared-memory must not be used"); ", so --shared-memory must not be used");
if (!Used.count("bulk-memory") && Config->PassiveSegments) if (!Used.count("bulk-memory") && Config->PassiveSegments)
error("'bulk-memory' feature must be used in order to emit passive " error("'bulk-memory' feature must be used in order to emit passive "
"segments"); "segments");
if (!Used.count("bulk-memory") && TLSUsed)
error("'bulk-memory' feature must be used in order to use thread-local "
"storage");
aheejinUnsubmitted
Done
ReplyInline Actions

Should we check for "mutable-global" feature too?

aheejin: Should we check for "mutable-global" feature too?
quantumAuthorUnsubmitted
Done
ReplyInline Actions

Do we need to? I thought it's always available since we use it for the stack pointer.

quantum: Do we need to? I thought it's always available since we use it for the stack pointer.
quantumAuthorUnsubmitted
Done
ReplyInline Actions

On second thought, the mutable-global feature is for import/export of mutable globals. TLS does not need to do this.

quantum: On second thought, the `mutable-global` feature is for import/export of mutable globals. TLS…
aheejinUnsubmitted
Done
ReplyInline Actions

Ah you're right.

aheejin: Ah you're right.
// Validate that used features are allowed in output // Validate that used features are allowed in output
if (!InferFeatures) { if (!InferFeatures) {
for (auto &Feature : Used.keys()) { for (auto &Feature : Used.keys()) {
if (!Out.TargetFeaturesSec->Features.count(Feature)) if (!Out.TargetFeaturesSec->Features.count(Feature))
error(Twine("Target feature '") + Feature + "' used by " + error(Twine("Target feature '") + Feature + "' used by " +
Used[Feature] + " is not allowed."); Used[Feature] + " is not allowed.");
} }
} }
▲ Show 20 Lines • Show All 64 Lines • ▼ Show 20 Lines for (Symbol *Sym : Symtab->getSymbols()) {
StringRef Name = Sym->getName(); StringRef Name = Sym->getName();
WasmExport Export; WasmExport Export;
if (auto *F = dyn_cast<DefinedFunction>(Sym)) { if (auto *F = dyn_cast<DefinedFunction>(Sym)) {
Export = {Name, WASM_EXTERNAL_FUNCTION, F->getFunctionIndex()}; Export = {Name, WASM_EXTERNAL_FUNCTION, F->getFunctionIndex()};
} else if (auto *G = dyn_cast<DefinedGlobal>(Sym)) { } else if (auto *G = dyn_cast<DefinedGlobal>(Sym)) {
// TODO(sbc): Remove this check once to mutable global proposal is // TODO(sbc): Remove this check once to mutable global proposal is
// implement in all major browsers. // implement in all major browsers.
// See: https://github.com/WebAssembly/mutable-global // See: https://github.com/WebAssembly/mutable-global
if (G->getGlobalType()->Mutable) { if (G->getGlobalType()->Mutable) {
// Only the __stack_pointer should ever be create as mutable. // Only __stack_pointer and __tls_base should ever be create as mutable.
aheejinUnsubmitted
Done
ReplyInline Actions

Now __tls_base is also mutable, I think we should fix the comment

aheejin: Now `__tls_base` is also mutable, I think we should fix the comment
quantumAuthorUnsubmitted
Done
ReplyInline Actions

Will do.

quantum: Will do.
aheejinUnsubmitted
Done
ReplyInline Actions

PIng. It doesn't look like fixed yet

aheejin: PIng. It doesn't look like fixed yet
quantumAuthorUnsubmitted
Done
ReplyInline Actions

Looks like the fix got rebasing against the change that made everything lowerCamelCase. Going to do it again.

quantum: Looks like the fix got rebasing against the change that made everything lowerCamelCase. Going…
assert(G == WasmSym::StackPointer); assert(G == WasmSym::StackPointer || G == WasmSym::TLSBase);
continue; continue;
} }
Export = {Name, WASM_EXTERNAL_GLOBAL, G->getGlobalIndex()}; Export = {Name, WASM_EXTERNAL_GLOBAL, G->getGlobalIndex()};
} else if (auto *E = dyn_cast<DefinedEvent>(Sym)) { } else if (auto *E = dyn_cast<DefinedEvent>(Sym)) {
Export = {Name, WASM_EXTERNAL_EVENT, E->getEventIndex()}; Export = {Name, WASM_EXTERNAL_EVENT, E->getEventIndex()};
} else { } else {
auto *D = cast<DefinedData>(Sym); auto *D = cast<DefinedData>(Sym);
Out.GlobalSec->DefinedFakeGlobals.emplace_back(D); Out.GlobalSec->DefinedFakeGlobals.emplace_back(D);
▲ Show 20 Lines • Show All 95 Lines • ▼ Show 20 Lines
static StringRef getOutputDataSegmentName(StringRef Name) { static StringRef getOutputDataSegmentName(StringRef Name) {
// With PIC code we currently only support a single data segment since // With PIC code we currently only support a single data segment since
// we only have a single __memory_base to use as our base address. // we only have a single __memory_base to use as our base address.
if (Config->Pic) if (Config->Pic)
return "data"; return "data";
if (!Config->MergeDataSegments) if (!Config->MergeDataSegments)
return Name; return Name;
if (Name.startswith(".text.")) if (Name.startswith(".text."))
return ".text"; return ".text";
sbc100Unsubmitted
Done
ReplyInline Actions

Maybe write this as a single conditional so its clear even without this comment?

sbc100: Maybe write this as a single conditional so its clear even without this comment?
quantumAuthorUnsubmitted
Done
ReplyInline Actions

Changed in latest diff.

quantum: Changed in latest diff.
aheejinUnsubmitted
Done
ReplyInline Actions

Has it been reflected? It says it has been changed but it doesn't look like it has

aheejin: Has it been reflected? It says it has been changed but it doesn't look like it has
quantumAuthorUnsubmitted
Done
ReplyInline Actions

I think you are commenting on an old version? It is a single condition on the latest version.

quantum: I think you are commenting on an old version? It is a single condition on the latest version.
if (Name.startswith(".data.")) if (Name.startswith(".data."))
return ".data"; return ".data";
if (Name.startswith(".bss.")) if (Name.startswith(".bss."))
return ".bss"; return ".bss";
if (Name.startswith(".rodata.")) if (Name.startswith(".rodata."))
return ".rodata"; return ".rodata";
if (Name.startswith(".tdata."))
return ".tdata";
// Merge .tbss into .tdata so that they share the same offsets.
if (Name.startswith(".tbss."))
return ".tdata";
tlivelyUnsubmitted
Done
ReplyInline Actions

Does this mean we can't control whether .tdata or .tbss comes first? Is that important for anything?

tlively: Does this mean we can't control whether .tdata or .tbss comes first? Is that important for…
quantumAuthorUnsubmitted
Done
ReplyInline Actions

Yes, it does mean that. The only reason why .tbss is supposed to be separate is so that its memory can just be zeroed whereas .tdata has to have the bytes stored in the program image. Currently, we just explicitly store the zero bytes, so this won't be a problem.

quantum: Yes, it does mean that. The only reason why .tbss is supposed to be separate is so that its…
tlivelyUnsubmitted
Done
ReplyInline Actions

It would be really great if we could find a way to elide the .bss 0 bytes as a code size optimization. Since we can't assume that the memory is already zeroed the way we can with passive segments, perhaps we can use a memory.fill instruction to zero the memory? Pursuing this in a follow-on CL should be fine.

tlively: It would be really great if we could find a way to elide the .bss 0 bytes as a code size…
quantumAuthorUnsubmitted
Done
ReplyInline Actions

Yes, this is a good idea for a follow-on CL.

quantum: Yes, this is a good idea for a follow-on CL.
return Name; return Name;
} }
void Writer::createOutputSegments() { void Writer::createOutputSegments() {
for (ObjFile *File : Symtab->ObjectFiles) { for (ObjFile *File : Symtab->ObjectFiles) {
for (InputSegment *Segment : File->Segments) { for (InputSegment *Segment : File->Segments) {
if (!Segment->Live) if (!Segment->Live)
continue; continue;
StringRef Name = getOutputDataSegmentName(Segment->getName()); StringRef Name = getOutputDataSegmentName(Segment->getName());
OutputSegment *&S = SegmentMap[Name]; OutputSegment *&S = SegmentMap[Name];
if (S == nullptr) { if (S == nullptr) {
LLVM_DEBUG(dbgs() << "new segment: " << Name << "\n"); LLVM_DEBUG(dbgs() << "new segment: " << Name << "\n");
S = make<OutputSegment>(Name, Segments.size()); S = make<OutputSegment>(Name, Segments.size());
if (Config->PassiveSegments) if (Config->PassiveSegments || Name == ".tdata")
S->InitFlags = WASM_SEGMENT_IS_PASSIVE; S->InitFlags = WASM_SEGMENT_IS_PASSIVE;
Segments.push_back(S); Segments.push_back(S);
} }
S->addInputSegment(Segment); S->addInputSegment(Segment);
LLVM_DEBUG(dbgs() << "added data: " << Name << ": " << S->Size << "\n"); LLVM_DEBUG(dbgs() << "added data: " << Name << ": " << S->Size << "\n");
} }
} }
} }
Show All 13 Linesvoid Writer::createInitMemoryFunction() {
LLVM_DEBUG(dbgs() << "createInitMemoryFunction\n"); LLVM_DEBUG(dbgs() << "createInitMemoryFunction\n");
std::string BodyContent; std::string BodyContent;
{ {
raw_string_ostream OS(BodyContent); raw_string_ostream OS(BodyContent);
writeUleb128(OS, 0, "num locals"); writeUleb128(OS, 0, "num locals");
// initialize passive data segments // initialize passive data segments
for (const OutputSegment *S : Segments) { for (const OutputSegment *S : Segments) {
if (S->InitFlags & WASM_SEGMENT_IS_PASSIVE) { if (S->InitFlags & WASM_SEGMENT_IS_PASSIVE && S->Name != ".tdata") {
// destination address // destination address
writeU8(OS, WASM_OPCODE_I32_CONST, "i32.const"); writeU8(OS, WASM_OPCODE_I32_CONST, "i32.const");
writeUleb128(OS, S->StartVA, "destination address"); writeUleb128(OS, S->StartVA, "destination address");
// source segment offset // source segment offset
writeU8(OS, WASM_OPCODE_I32_CONST, "i32.const"); writeU8(OS, WASM_OPCODE_I32_CONST, "i32.const");
writeUleb128(OS, 0, "segment offset"); writeUleb128(OS, 0, "segment offset");
// memory region size // memory region size
writeU8(OS, WASM_OPCODE_I32_CONST, "i32.const"); writeU8(OS, WASM_OPCODE_I32_CONST, "i32.const");
▲ Show 20 Lines • Show All 65 Lines • ▼ Show 20 Lines for (const WasmInitEntry &F : InitFunctions) {
writeUleb128(OS, F.Sym->getFunctionIndex(), "function index"); writeUleb128(OS, F.Sym->getFunctionIndex(), "function index");
} }
writeU8(OS, WASM_OPCODE_END, "END"); writeU8(OS, WASM_OPCODE_END, "END");
} }
CreateFunction(WasmSym::CallCtors, BodyContent); CreateFunction(WasmSym::CallCtors, BodyContent);
} }
void Writer::createInitTLSFunction() {
if (!WasmSym::InitTLS || !WasmSym::InitTLS->isLive())
return;
std::string BodyContent;
{
aheejinUnsubmitted
Done
ReplyInline Actions

Any reason for the new block scope?

aheejin: Any reason for the new block scope?
quantumAuthorUnsubmitted
Done
ReplyInline Actions

Yes, the raw_string_ostream must destruct by the end of the scope. This is the pattern used in other functions above.

quantum: Yes, the `raw_string_ostream` must destruct by the end of the scope. This is the pattern used…
raw_string_ostream OS(BodyContent);
OutputSegment *TLSSeg = nullptr;
for (auto *Seg : Segments) {
if (Seg->Name == ".tdata")
TLSSeg = Seg;
break;
}
aheejinUnsubmitted
Done
ReplyInline Actions

Is it guaranteed that there's only one TLS segment?

aheejin: Is it guaranteed that there's only one TLS segment?
quantumAuthorUnsubmitted
Done
ReplyInline Actions

Yes, all the TLS input segments will be merged into .tdata.

quantum: Yes, all the TLS input segments will be merged into `.tdata`.
tlivelyUnsubmitted
Done
ReplyInline Actions

--no-merge-data-segments!

tlively: --no-merge-data-segments!
quantumAuthorUnsubmitted
Done
ReplyInline Actions

I am making --no-merge-data-segments merge TLS segments anyway.

quantum: I am making `--no-merge-data-segments` merge TLS segments anyway.
if (TLSSeg) {
writeUleb128(OS, 0, "num locals");
writeU8(OS, WASM_OPCODE_LOCAL_GET, "local.get");
writeUleb128(OS, 0, "local index");
writeU8(OS, WASM_OPCODE_GLOBAL_SET, "global.set");
writeUleb128(OS, WasmSym::TLSBase->getGlobalIndex(), "global index");
writeU8(OS, WASM_OPCODE_LOCAL_GET, "local.get");
writeUleb128(OS, 0, "local index");
writeU8(OS, WASM_OPCODE_I32_CONST, "i32.const");
writeUleb128(OS, 0, "segment offset");
writeU8(OS, WASM_OPCODE_I32_CONST, "i32.const");
writeUleb128(OS, TLSSeg->Size, "memory region size");
writeU8(OS, WASM_OPCODE_MISC_PREFIX, "bulk-memory prefix");
writeUleb128(OS, WASM_OPCODE_MEMORY_INIT, "MEMORY.INIT");
writeUleb128(OS, TLSSeg->Index, "segment index immediate");
writeU8(OS, 0, "memory index immediate");
writeU8(OS, WASM_OPCODE_END, "end function");
} else {
writeUleb128(OS, 0, "num locals");
writeU8(OS, WASM_OPCODE_END, "end function");
tlivelyUnsubmitted
Done
ReplyInline Actions

You could avoid duplicating these lines by making them unconditional.

tlively: You could avoid duplicating these lines by making them unconditional.
quantumAuthorUnsubmitted
Done
ReplyInline Actions

Right.

quantum: Right.
}
}
CreateFunction(WasmSym::InitTLS, BodyContent);
}
// Populate InitFunctions vector with init functions from all input objects. // Populate InitFunctions vector with init functions from all input objects.
// This is then used either when creating the output linking section or to // This is then used either when creating the output linking section or to
// synthesize the "__wasm_call_ctors" function. // synthesize the "__wasm_call_ctors" function.
void Writer::calculateInitFunctions() { void Writer::calculateInitFunctions() {
if (!Config->Relocatable && !WasmSym::CallCtors->isLive()) if (!Config->Relocatable && !WasmSym::CallCtors->isLive())
return; return;
for (ObjFile *File : Symtab->ObjectFiles) { for (ObjFile *File : Symtab->ObjectFiles) {
▲ Show 20 Lines • Show All 78 Lines • ▼ Show 20 Lines if (Config->PassiveSegments)
createInitMemoryFunction(); createInitMemoryFunction();
if (Config->Pic) if (Config->Pic)
createApplyRelocationsFunction(); createApplyRelocationsFunction();
createCallCtorsFunction(); createCallCtorsFunction();
// Make sure we have resolved all symbols. // Make sure we have resolved all symbols.
if (!Config->AllowUndefined) if (!Config->AllowUndefined)
Symtab->reportRemainingUndefines(); Symtab->reportRemainingUndefines();
}
createInitTLSFunction();
tlivelyUnsubmitted
Done
ReplyInline Actions

Can you remind me how the InitTLSFunction interacts with relocatable code? I'm wondering if this should be called up in the condition with the other synthetic functions.

tlively: Can you remind me how the InitTLSFunction interacts with relocatable code? I'm wondering if…
quantumAuthorUnsubmitted
Done
ReplyInline Actions

I don't think it should. __wasm_init_tls initializes the TLS block for the current module only, so every shared library needs to have its own __wasm_init_tls.

quantum: I don't think it should. `__wasm_init_tls` initializes the TLS block for the current module…
if (errorCount()) if (errorCount())
return; return;
}
log("-- calculateTypes"); log("-- calculateTypes");
calculateTypes(); calculateTypes();
log("-- calculateExports"); log("-- calculateExports");
calculateExports(); calculateExports();
log("-- calculateCustomSections"); log("-- calculateCustomSections");
calculateCustomSections(); calculateCustomSections();
log("-- populateSymtab"); log("-- populateSymtab");
▲ Show 20 Lines • Show All 59 Lines • Show Last 20 Lines