Skip to content

Commit

Permalink
[WebAssembly] Add COMDAT support
Browse files Browse the repository at this point in the history 
See https://bugs.llvm.org/show_bug.cgi?id=35533, and D40844

Things covered:
* Removing duplicate data segments (as determined by COMDATs emitted
  by the frontend)
* Removing duplicate globals and functions in COMDATs
* Checking that each time a COMDAT is seen it has the same symbols
  as at other times (ie it's a stronger check than simply giving all
  the symbols in the COMDAT weak linkage)

Patch by Nicholas Wilson!

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

llvm-svn: 322415
  • Loading branch information
@sbc100
sbc100 committed Jan 12, 2018
1 parent d103612 commit e0f6fcd
Show file tree
Hide file tree
Showing 10 changed files with 251 additions and 23 deletions.
11 changes: 11 additions & 0 deletions lld/test/wasm/Inputs/comdat1.ll
View file
@@ -0,0 +1,11 @@
$inlineFn = comdat any
@constantData = weak_odr constant [3 x i8] c"abc", comdat($inlineFn)
define linkonce_odr i32 @inlineFn() comdat {
entry:
ret i32 ptrtoint ([3 x i8]* @constantData to i32)
}

define i32 @callInline1() {
entry:
ret i32 ptrtoint (i32 ()* @inlineFn to i32)
}
11 changes: 11 additions & 0 deletions lld/test/wasm/Inputs/comdat2.ll
View file
@@ -0,0 +1,11 @@
$inlineFn = comdat any
@constantData = weak_odr constant [3 x i8] c"abc", comdat($inlineFn)
define linkonce_odr i32 @inlineFn() comdat {
entry:
ret i32 ptrtoint ([3 x i8]* @constantData to i32)
}

define i32 @callInline2() {
entry:
ret i32 ptrtoint (i32 ()* @inlineFn to i32)
}
70 changes: 70 additions & 0 deletions lld/test/wasm/comdats.ll
View file
@@ -0,0 +1,70 @@
; RUN: llc -filetype=obj -mtriple=wasm32-unknown-uknown-wasm %p/Inputs/comdat1.ll -o %t1.o
; RUN: llc -filetype=obj -mtriple=wasm32-unknown-uknown-wasm %p/Inputs/comdat2.ll -o %t2.o
; RUN: llc -filetype=obj -mtriple=wasm32-unknown-uknown-wasm %s -o %t.o
; RUN: lld -flavor wasm -o %t.wasm %t.o %t1.o %t2.o
; RUN: obj2yaml %t.wasm | FileCheck %s

declare i32 @inlineFn()

define void @_start() local_unnamed_addr {
entry:
%call = call i32 @inlineFn()
ret void
}

; CHECK: - Type: GLOBAL
; CHECK-NEXT: Globals:
; CHECK-NEXT: - Index: 0
; CHECK-NEXT: Type: I32
; CHECK-NEXT: Mutable: true
; CHECK-NEXT: InitExpr:
; CHECK-NEXT: Opcode: I32_CONST
; CHECK-NEXT: Value: 66576
; CHECK-NEXT: - Type: EXPORT
; CHECK-NEXT: Exports:
; CHECK-NEXT: - Name: memory
; CHECK-NEXT: Kind: MEMORY
; CHECK-NEXT: Index: 0
; CHECK-NEXT: - Name: _start
; CHECK-NEXT: Kind: FUNCTION
; CHECK-NEXT: Index: 0
; CHECK-NEXT: - Name: inlineFn
; CHECK-NEXT: Kind: FUNCTION
; CHECK-NEXT: Index: 1
; CHECK-NEXT: - Name: callInline1
; CHECK-NEXT: Kind: FUNCTION
; CHECK-NEXT: Index: 2
; CHECK-NEXT: - Name: callInline2
; CHECK-NEXT: Kind: FUNCTION
; CHECK-NEXT: Index: 3
; CHECK-NEXT: - Type: ELEM
; CHECK-NEXT: Segments:
; CHECK-NEXT: - Offset:
; CHECK-NEXT: Opcode: I32_CONST
; CHECK-NEXT: Value: 1
; CHECK-NEXT: Functions: [ 1 ]
; CHECK-NEXT: - Type: CODE
; CHECK-NEXT: Functions:
; CHECK-NEXT: - Index: 0
; CHECK-NEXT: Locals:
; CHECK-NEXT: Body: 1081808080001A0B
; CHECK-NEXT: - Index: 1
; CHECK-NEXT: Locals:
; CHECK-NEXT: Body: 4180888080000B
; CHECK-NEXT: - Index: 2
; CHECK-NEXT: Locals:
; CHECK-NEXT: Body: 4181808080000B
; CHECK-NEXT: - Index: 3
; CHECK-NEXT: Locals:
; CHECK-NEXT: Body: 4181808080000B
; CHECK-NEXT: - Index: 4
; CHECK-NEXT: Locals:
; CHECK-NEXT: Body: 0B
; CHECK-NEXT: - Type: DATA
; CHECK-NEXT: Segments:
; CHECK-NEXT: - SectionOffset: 7
; CHECK-NEXT: MemoryIndex: 0
; CHECK-NEXT: Offset:
; CHECK-NEXT: Opcode: I32_CONST
; CHECK-NEXT: Value: 1024
; CHECK-NEXT: Content: '616263'
64 changes: 53 additions & 11 deletions lld/test/wasm/relocatable.ll
View file
Expand Up @@ -17,6 +17,13 @@ declare i32 @foo_import() local_unnamed_addr
@func_addr2 = hidden global i32()* @foo_import, align 4
@data_addr1 = hidden global i64* @data_import, align 8

$func_comdat = comdat any
@data_comdat = weak_odr constant [3 x i8] c"abc", comdat($func_comdat)
define linkonce_odr i32 @func_comdat() comdat {
entry:
ret i32 ptrtoint ([3 x i8]* @data_comdat to i32)
}

; CHECK: --- !WASM
; CHECK-NEXT: FileHeader:
; CHECK-NEXT: Version: 0x00000001
Expand Down Expand Up @@ -49,7 +56,7 @@ declare i32 @foo_import() local_unnamed_addr
; CHECK-NEXT: GlobalType: I32
; CHECK-NEXT: GlobalMutable: false
; CHECK-NEXT: - Type: FUNCTION
; CHECK-NEXT: FunctionTypes: [ 0, 2 ]
; CHECK-NEXT: FunctionTypes: [ 0, 2, 2 ]
; CHECK-NEXT: - Type: TABLE
; CHECK-NEXT: Tables:
; CHECK-NEXT: - ElemType: ANYFUNC
Expand All @@ -73,18 +80,24 @@ declare i32 @foo_import() local_unnamed_addr
; CHECK-NEXT: Mutable: false
; CHECK-NEXT: InitExpr:
; CHECK-NEXT: Opcode: I32_CONST
; CHECK-NEXT: Value: 8
; CHECK-NEXT: Value: 20
; CHECK-NEXT: - Index: 3
; CHECK-NEXT: Type: I32
; CHECK-NEXT: Mutable: false
; CHECK-NEXT: InitExpr:
; CHECK-NEXT: Opcode: I32_CONST
; CHECK-NEXT: Value: 12
; CHECK-NEXT: Value: 8
; CHECK-NEXT: - Index: 4
; CHECK-NEXT: Type: I32
; CHECK-NEXT: Mutable: false
; CHECK-NEXT: InitExpr:
; CHECK-NEXT: Opcode: I32_CONST
; CHECK-NEXT: Value: 12
; CHECK-NEXT: - Index: 5
; CHECK-NEXT: Type: I32
; CHECK-NEXT: Mutable: false
; CHECK-NEXT: InitExpr:
; CHECK-NEXT: Opcode: I32_CONST
; CHECK-NEXT: Value: 16
; CHECK-NEXT: - Type: EXPORT
; CHECK-NEXT: Exports:
Expand All @@ -94,18 +107,24 @@ declare i32 @foo_import() local_unnamed_addr
; CHECK-NEXT: - Name: my_func
; CHECK-NEXT: Kind: FUNCTION
; CHECK-NEXT: Index: 3
; CHECK-NEXT: - Name: func_comdat
; CHECK-NEXT: Kind: FUNCTION
; CHECK-NEXT: Index: 4
; CHECK-NEXT: - Name: hello_str
; CHECK-NEXT: Kind: GLOBAL
; CHECK-NEXT: Index: 1
; CHECK-NEXT: - Name: func_addr1
; CHECK-NEXT: - Name: data_comdat
; CHECK-NEXT: Kind: GLOBAL
; CHECK-NEXT: Index: 2
; CHECK-NEXT: - Name: func_addr2
; CHECK-NEXT: - Name: func_addr1
; CHECK-NEXT: Kind: GLOBAL
; CHECK-NEXT: Index: 3
; CHECK-NEXT: - Name: data_addr1
; CHECK-NEXT: - Name: func_addr2
; CHECK-NEXT: Kind: GLOBAL
; CHECK-NEXT: Index: 4
; CHECK-NEXT: - Name: data_addr1
; CHECK-NEXT: Kind: GLOBAL
; CHECK-NEXT: Index: 5
; CHECK-NEXT: - Type: ELEM
; CHECK-NEXT: Segments:
; CHECK-NEXT: - Offset:
Expand All @@ -123,13 +142,19 @@ declare i32 @foo_import() local_unnamed_addr
; CHECK-NEXT: - Type: R_WEBASSEMBLY_FUNCTION_INDEX_LEB
; CHECK-NEXT: Index: 1
; CHECK-NEXT: Offset: 0x00000013
; CHECK-NEXT: - Type: R_WEBASSEMBLY_MEMORY_ADDR_SLEB
; CHECK-NEXT: Index: 2
; CHECK-NEXT: Offset: 0x0000001F
; CHECK-NEXT: Functions:
; CHECK-NEXT: - Index: 2
; CHECK-NEXT: Locals:
; CHECK-NEXT: Body: 4180808080001080808080000B
; CHECK-NEXT: - Index: 3
; CHECK-NEXT: Locals:
; CHECK-NEXT: Body: 1081808080001A41010B
; CHECK-NEXT: - Index: 4
; CHECK-NEXT: Locals:
; CHECK-NEXT: Body: 4194808080000B
; CHECK-NEXT: - Type: DATA
; CHECK-NEXT: Relocations:
; CHECK-NEXT: - Type: R_WEBASSEMBLY_TABLE_INDEX_I32
Expand Down Expand Up @@ -166,26 +191,43 @@ declare i32 @foo_import() local_unnamed_addr
; CHECK-NEXT: Opcode: I32_CONST
; CHECK-NEXT: Value: 16
; CHECK-NEXT: Content: '00000000'
; CHECK-NEXT: - SectionOffset: 45
; CHECK-NEXT: MemoryIndex: 0
; CHECK-NEXT: Offset:
; CHECK-NEXT: Opcode: I32_CONST
; CHECK-NEXT: Value: 20
; CHECK-NEXT: Content: '616263'
; CHECK-NEXT: - Type: CUSTOM
; CHECK-NEXT: Name: linking
; CHECK-NEXT: DataSize: 20
; CHECK-NEXT: DataSize: 23
; CHECK-NEXT: SegmentInfo:
; CHECK-NEXT: - Index: 0
; CHECK-NEXT: Name: .rodata.hello_str
; CHECK-NEXT: Alignment: 1
; CHECK-NEXT: Flags: [ ]
; CHECK-NEXT: Flags: [ ]
; CHECK-NEXT: - Index: 1
; CHECK-NEXT: Name: .data.func_addr1
; CHECK-NEXT: Alignment: 4
; CHECK-NEXT: Flags: [ ]
; CHECK-NEXT: Flags: [ ]
; CHECK-NEXT: - Index: 2
; CHECK-NEXT: Name: .data.func_addr2
; CHECK-NEXT: Alignment: 4
; CHECK-NEXT: Flags: [ ]
; CHECK-NEXT: Flags: [ ]
; CHECK-NEXT: - Index: 3
; CHECK-NEXT: Name: .data.data_addr1
; CHECK-NEXT: Alignment: 8
; CHECK-NEXT: Flags: [ ]
; CHECK-NEXT: Flags: [ ]
; CHECK-NEXT: - Index: 4
; CHECK-NEXT: Name: .rodata.data_comdat
; CHECK-NEXT: Alignment: 1
; CHECK-NEXT: Flags: [ ]
; CHECK-NEXT: Comdats:
; CHECK-NEXT: - Name: func_comdat
; CHECK-NEXT: Entries:
; CHECK-NEXT: - Kind: FUNCTION
; CHECK-NEXT: Index: 4
; CHECK-NEXT: - Kind: DATA
; CHECK-NEXT: Index: 4
; CHECK-NEXT: - Type: CUSTOM
; CHECK-NEXT: Name: name
; CHECK-NEXT: FunctionNames:
Expand Down
7 changes: 6 additions & 1 deletion lld/wasm/InputChunks.h
View file
Expand Up @@ -47,6 +47,9 @@ class InputChunk {

uint32_t getOutputOffset() const { return OutputOffset; }

virtual StringRef getComdat() const = 0;

bool Discarded = false;
std::vector<OutputRelocation> OutRelocations;

protected:
Expand Down Expand Up @@ -93,13 +96,15 @@ class InputSegment : public InputChunk {
uint32_t startVA() const { return Segment.Data.Offset.Value.Int32; }
uint32_t endVA() const { return startVA() + getSize(); }
StringRef getName() const { return Segment.Data.Name; }
StringRef getComdat() const override { return Segment.Data.Comdat; }

int32_t OutputSegmentOffset = 0;

protected:
uint32_t getInputSectionOffset() const override {
return Segment.SectionOffset;
}

const WasmSegment &Segment;
const OutputSegment *OutputSeg = nullptr;
};
Expand All @@ -116,7 +121,7 @@ class InputFunction : public InputChunk {
const uint8_t *getData() const override {
return File->CodeSection->Content.data() + getInputSectionOffset();
}

StringRef getComdat() const override { return Function->Comdat; }
uint32_t getOutputIndex() const { return OutputIndex.getValue(); };
bool hasOutputIndex() const { return OutputIndex.hasValue(); };
void setOutputIndex(uint32_t Index);
Expand Down
49 changes: 38 additions & 11 deletions lld/wasm/InputFiles.cpp
View file
Expand Up @@ -170,6 +170,11 @@ InputFunction *ObjFile::getFunction(const WasmSymbol &Sym) const {
return Functions[FunctionIndex];
}

bool ObjFile::isExcludedByComdat(InputChunk *Chunk) const {
StringRef Comdat = Chunk->getComdat();
return !Comdat.empty() && Symtab->findComdat(Comdat) != this;
}

void ObjFile::initializeSymbols() {
Symbols.reserve(WasmObj->getNumberOfSymbols());

Expand All @@ -187,15 +192,23 @@ void ObjFile::initializeSymbols() {
FunctionSymbols.resize(NumFunctionImports + WasmObj->functions().size());
GlobalSymbols.resize(NumGlobalImports + WasmObj->globals().size());

ArrayRef<WasmFunction> Funcs = WasmObj->functions();
ArrayRef<uint32_t> FuncTypes = WasmObj->functionTypes();
ArrayRef<WasmSignature> Types = WasmObj->types();
ArrayRef<WasmGlobal> Globals = WasmObj->globals();

for (const auto &C : WasmObj->comdats())
Symtab->addComdat(C, this);

FunctionSymbols.resize(NumFunctionImports + Funcs.size());
GlobalSymbols.resize(NumGlobalImports + Globals.size());

for (const WasmSegment &S : WasmObj->dataSegments()) {
InputSegment *Seg = make<InputSegment>(S, this);
Seg->copyRelocations(*DataSection);
Segments.emplace_back(Seg);
}

ArrayRef<WasmFunction> Funcs = WasmObj->functions();
ArrayRef<uint32_t> FuncTypes = WasmObj->functionTypes();
ArrayRef<WasmSignature> Types = WasmObj->types();
for (size_t I = 0; I < Funcs.size(); ++I) {
const WasmFunction &Func = Funcs[I];
const WasmSignature &Sig = Types[FuncTypes[I]];
Expand All @@ -210,21 +223,35 @@ void ObjFile::initializeSymbols() {
const WasmSymbol &WasmSym = WasmObj->getWasmSymbol(Sym.getRawDataRefImpl());
Symbol *S;
switch (WasmSym.Type) {
case WasmSymbol::SymbolType::FUNCTION_EXPORT: {
InputFunction *Function = getFunction(WasmSym);
if (!isExcludedByComdat(Function)) {
S = createDefined(WasmSym, Symbol::Kind::DefinedFunctionKind, nullptr,
Function);
break;
} else {
Function->Discarded = true;
LLVM_FALLTHROUGH; // Exclude function, and add the symbol as undefined
}
}
case WasmSymbol::SymbolType::FUNCTION_IMPORT:
S = createUndefined(WasmSym, Symbol::Kind::UndefinedFunctionKind,
getFunctionSig(WasmSym));
break;
case WasmSymbol::SymbolType::GLOBAL_EXPORT: {
InputSegment *Segment = getSegment(WasmSym);
if (!isExcludedByComdat(Segment)) {
S = createDefined(WasmSym, Symbol::Kind::DefinedGlobalKind,
Segment, nullptr, getGlobalValue(WasmSym));
break;
} else {
Segment->Discarded = true;
LLVM_FALLTHROUGH; // Exclude global, and add the symbol as undefined
}
}
case WasmSymbol::SymbolType::GLOBAL_IMPORT:
S = createUndefined(WasmSym, Symbol::Kind::UndefinedGlobalKind);
break;
case WasmSymbol::SymbolType::GLOBAL_EXPORT:
S = createDefined(WasmSym, Symbol::Kind::DefinedGlobalKind,
getSegment(WasmSym), nullptr, getGlobalValue(WasmSym));
break;
case WasmSymbol::SymbolType::FUNCTION_EXPORT:
S = createDefined(WasmSym, Symbol::Kind::DefinedFunctionKind, nullptr,
getFunction(WasmSym));
break;
case WasmSymbol::SymbolType::DEBUG_FUNCTION_NAME:
// These are for debugging only, no need to create linker symbols for them
continue;
Expand Down
2 changes: 2 additions & 0 deletions lld/wasm/InputFiles.h
View file
Expand Up @@ -33,6 +33,7 @@ using llvm::wasm::WasmRelocation;
namespace lld {
namespace wasm {

class InputChunk;
class InputFunction;
class InputSegment;

Expand Down Expand Up @@ -122,6 +123,7 @@ class ObjFile : public InputFile {
const WasmSignature *getFunctionSig(const WasmSymbol &Sym) const;
uint32_t getGlobalValue(const WasmSymbol &Sym) const;
InputFunction *getFunction(const WasmSymbol &Sym) const;
bool isExcludedByComdat(InputChunk *Chunk) const;

// List of all symbols referenced or defined by this file.
std::vector<Symbol *> Symbols;
Expand Down
16 changes: 16 additions & 0 deletions lld/wasm/SymbolTable.cpp
View file
Expand Up @@ -237,3 +237,19 @@ void SymbolTable::addLazy(ArchiveFile *F, const Archive::Symbol *Sym) {
F->addMember(Sym);
}
}

bool SymbolTable::addComdat(StringRef Name, ObjFile *F) {
DEBUG(dbgs() << "addComdat: " << Name << "\n");
ObjFile *&File = ComdatMap[CachedHashStringRef(Name)];
if (File) {
DEBUG(dbgs() << "COMDAT already defined\n");
return false;
}
File = F;
return true;
}

ObjFile *SymbolTable::findComdat(StringRef Name) const {
auto It = ComdatMap.find(CachedHashStringRef(Name));
return It == ComdatMap.end() ? nullptr : It->second;
}

0 comments on commit e0f6fcd

Please sign in to comment.