Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use absolute memory addresses for emasm string indexes. #2408

Merged
merged 2 commits into from
Oct 31, 2019
Merged

Use absolute memory addresses for emasm string indexes. #2408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
865545d
Use absolute memory addresses for emasm string indexes.
sbc100 Oct 30, 2019
ba6a94a
fix
sbc100 Oct 31, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 36 additions & 25 deletions src/wasm/wasm-emscripten.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -653,8 +653,7 @@ const char* stringAtAddr(Module& wasm,

std::string codeForConstAddr(Module& wasm,
std::vector<Address> const& segmentOffsets,
Const* addrConst) {
auto address = addrConst->value.geti32();
int32_t address) {
const char* str = stringAtAddr(wasm, segmentOffsets, address);
if (!str) {
// If we can't find the segment corresponding with the address, then we
Expand Down Expand Up @@ -710,7 +709,8 @@ struct AsmConstWalker : public LinearExecutionWalker<AsmConstWalker> {

private:
std::string fixupName(Name& name, std::string baseSig, Proxying proxy);
AsmConst& createAsmConst(std::string code, std::string sig, Name name);
AsmConst&
createAsmConst(uint32_t id, std::string code, std::string sig, Name name);
std::string asmConstSig(std::string baseSig);
Name nameForImportWithSig(std::string sig, Proxying proxy);
void queueImport(Name importName, std::string baseSig);
Expand Down Expand Up @@ -757,29 +757,38 @@ void AsmConstWalker::visitCall(Call* curr) {
<< ".\nThis might be caused by aggressive compiler "
"transformations. Consider using EM_JS instead.";
}
} else if (auto* value = arg->dynCast<Binary>()) {
// In the dynamic linking case the address of the string constant
// is the result of adding its offset to __memory_base.
// In this case are only looking for the offset with the data segment so
// the RHS of the addition is just what we want.
assert(value->op == AddInt32);
arg = value->right;
} else {
if (!value) {
Fatal() << "Unexpected arg0 type (" << getExpressionName(arg)
<< ") in call to: " << importName;
continue;
}

if (auto* setlocal = arg->dynCast<LocalSet>()) {
// The argument may be a local.tee, in which case we take first child
// which is the value being copied into the local.
if (setlocal->isTee()) {
arg = setlocal->value;
continue;
}
}

if (auto* bin = arg->dynCast<Binary>()) {
if (bin->op == AddInt32) {
// In the dynamic linking case the address of the string constant
// is the result of adding its offset to __memory_base.
// In this case are only looking for the offset from __memory_base
// the RHS of the addition is just what we want.
arg = bin->right;
continue;
}
}

Fatal() << "Unexpected arg0 type (" << getExpressionName(arg)
<< ") in call to: " << importName;
}

auto* value = arg->cast<Const>();
auto code = codeForConstAddr(wasm, segmentOffsets, value);
auto& asmConst = createAsmConst(code, sig, importName);
int32_t address = value->value.geti32();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason codeForConstAddr changed from taking a Const* to an int32_t? It seems like this was a partial change that didn't make it in to the final PR, so the effect was that we just outlined the value->value.geti32() bit.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I outlines it because it need to use that address in two places now. Maybe I should just have both consumers call ->value.geti32()?

auto code = codeForConstAddr(wasm, segmentOffsets, address);
auto& asmConst = createAsmConst(address, code, sig, importName);
fixupName(curr->target, baseSig, asmConst.proxy);

// Replace the first argument to the call with a Const index
Builder builder(wasm);
curr->operands[0] = builder.makeConst(Literal(asmConst.id));
}

Proxying AsmConstWalker::proxyType(Name name) {
Expand Down Expand Up @@ -826,14 +835,15 @@ AsmConstWalker::fixupName(Name& name, std::string baseSig, Proxying proxy) {
return sig;
}

AsmConstWalker::AsmConst&
AsmConstWalker::createAsmConst(std::string code, std::string sig, Name name) {
AsmConstWalker::AsmConst& AsmConstWalker::createAsmConst(uint32_t id,
std::string code,
std::string sig,
Name name) {
if (asmConsts.count(code) == 0) {
AsmConst asmConst;
asmConst.id = asmConsts.size();
asmConst.id = id;
asmConst.sigs.insert(sig);
asmConst.proxy = proxyType(name);

asmConsts[code] = asmConst;
}
return asmConsts[code];
Expand Down Expand Up @@ -918,7 +928,8 @@ struct EmJsWalker : public PostWalker<EmJsWalker> {
Fatal() << "Unexpected generated __em_js__ function body: " << curr->name;
}
auto* addrConst = consts.list[0];
auto code = codeForConstAddr(wasm, segmentOffsets, addrConst);
int32_t address = addrConst->value.geti32();
auto code = codeForConstAddr(wasm, segmentOffsets, address);
codeByName[funcName] = code;
}
};
Expand Down
12 changes: 6 additions & 6 deletions test/lld/em_asm.wast.mem.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
)
(drop
(call $emscripten_asm_const_iii
(i32.const 0)
(i32.const 568)
(i32.add
(local.get $0)
(i32.const 24)
Expand All @@ -69,7 +69,7 @@
)
(local.tee $1
(call $emscripten_asm_const_iii
(i32.const 1)
(i32.const 601)
(i32.add
(local.get $0)
(i32.const 24)
Expand All @@ -87,7 +87,7 @@
)
(drop
(call $emscripten_asm_const_iii
(i32.const 2)
(i32.const 621)
(i32.add
(local.get $0)
(i32.const 24)
Expand Down Expand Up @@ -226,9 +226,9 @@
--BEGIN METADATA --
{
"asmConsts": {
"2": ["{ Module.print(\"Got \" + $0); }", ["iii"], [""]],
"0": ["{ Module.print(\"Hello world\"); }", ["iii"], [""]],
"1": ["{ return $0 + $1; }", ["iii"], [""]]
"621": ["{ Module.print(\"Got \" + $0); }", ["iii"], [""]],
"568": ["{ Module.print(\"Hello world\"); }", ["iii"], [""]],
"601": ["{ return $0 + $1; }", ["iii"], [""]]
},
"staticBump": 84,
"tableSize": 1,
Expand Down
12 changes: 6 additions & 6 deletions test/lld/em_asm.wast.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
)
(drop
(call $emscripten_asm_const_iii
(i32.const 0)
(i32.const 568)
(i32.add
(local.get $0)
(i32.const 24)
Expand All @@ -70,7 +70,7 @@
)
(local.tee $1
(call $emscripten_asm_const_iii
(i32.const 1)
(i32.const 601)
(i32.add
(local.get $0)
(i32.const 24)
Expand All @@ -88,7 +88,7 @@
)
(drop
(call $emscripten_asm_const_iii
(i32.const 2)
(i32.const 621)
(i32.add
(local.get $0)
(i32.const 24)
Expand Down Expand Up @@ -227,9 +227,9 @@
--BEGIN METADATA --
{
"asmConsts": {
"2": ["{ Module.print(\"Got \" + $0); }", ["iii"], [""]],
"0": ["{ Module.print(\"Hello world\"); }", ["iii"], [""]],
"1": ["{ return $0 + $1; }", ["iii"], [""]]
"621": ["{ Module.print(\"Got \" + $0); }", ["iii"], [""]],
"568": ["{ Module.print(\"Hello world\"); }", ["iii"], [""]],
"601": ["{ return $0 + $1; }", ["iii"], [""]]
},
"staticBump": 84,
"tableSize": 1,
Expand Down
12 changes: 6 additions & 6 deletions test/lld/em_asm_O0.wast.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
(local $t2 i32)
(drop
(call $emscripten_asm_const_i
(i32.const 0)
(i32.const 568)
)
)
(local.set $t1
Expand All @@ -41,9 +41,9 @@
)
(drop
(call $emscripten_asm_const_ii
(i32.const 2)
(local.get $t1)
(call $emscripten_asm_const_iii
(i32.const 1)
(local.get $t2)
(i32.const 13)
(i32.const 27)
)
Expand Down Expand Up @@ -87,9 +87,9 @@
--BEGIN METADATA --
{
"asmConsts": {
"2": ["{ Module.print(\"Got \" + $0); }", ["ii"], [""]],
"0": ["{ Module.print(\"Hello world\"); }", ["i"], [""]],
"1": ["{ return $0 + $1; }", ["iii"], [""]]
"621": ["{ Module.print(\"Got \" + $0); }", ["ii"], [""]],
"568": ["{ Module.print(\"Hello world\"); }", ["i"], [""]],
"601": ["{ return $0 + $1; }", ["iii"], [""]]
},
"staticBump": 84,
"tableSize": 1,
Expand Down
12 changes: 6 additions & 6 deletions test/lld/em_asm_main_thread.wast.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
)
(drop
(call $emscripten_asm_const_sync_on_main_thread_iii
(i32.const 0)
(i32.const 568)
(i32.add
(local.get $0)
(i32.const 24)
Expand All @@ -70,7 +70,7 @@
)
(local.tee $1
(call $emscripten_asm_const_sync_on_main_thread_iii
(i32.const 1)
(i32.const 601)
(i32.add
(local.get $0)
(i32.const 24)
Expand All @@ -88,7 +88,7 @@
)
(drop
(call $emscripten_asm_const_sync_on_main_thread_iii
(i32.const 2)
(i32.const 621)
(i32.add
(local.get $0)
(i32.const 24)
Expand Down Expand Up @@ -227,9 +227,9 @@
--BEGIN METADATA --
{
"asmConsts": {
"2": ["{ Module.print(\"Got \" + $0); }", ["iii"], ["sync_on_main_thread_"]],
"0": ["{ Module.print(\"Hello world\"); }", ["iii"], ["sync_on_main_thread_"]],
"1": ["{ return $0 + $1; }", ["iii"], ["sync_on_main_thread_"]]
"621": ["{ Module.print(\"Got \" + $0); }", ["iii"], ["sync_on_main_thread_"]],
"568": ["{ Module.print(\"Hello world\"); }", ["iii"], ["sync_on_main_thread_"]],
"601": ["{ return $0 + $1; }", ["iii"], ["sync_on_main_thread_"]]
},
"staticBump": 84,
"tableSize": 1,
Expand Down
21 changes: 16 additions & 5 deletions test/lld/em_asm_shared.wast.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,12 @@
)
(drop
(call $emscripten_asm_const_iii
(i32.const 0)
(i32.add
(local.tee $1
(global.get $gimport$3)
)
(i32.const 0)
)
(i32.add
(local.get $0)
(i32.const 24)
Expand All @@ -75,7 +80,10 @@
)
(local.tee $2
(call $emscripten_asm_const_iii
(i32.const 1)
(i32.add
(local.get $1)
(i32.const 33)
)
(i32.add
(local.get $0)
(i32.const 24)
Expand All @@ -93,7 +101,10 @@
)
(drop
(call $emscripten_asm_const_iii
(i32.const 2)
(i32.add
(local.get $1)
(i32.const 53)
)
(i32.add
(local.get $0)
(i32.const 24)
Expand Down Expand Up @@ -207,9 +218,9 @@
--BEGIN METADATA --
{
"asmConsts": {
"2": ["{ Module.print(\"Got \" + $0); }", ["iii"], [""]],
"53": ["{ Module.print(\"Got \" + $0); }", ["iii"], [""]],
"0": ["{ Module.print(\"Hello world\"); }", ["iii"], [""]],
"1": ["{ return $0 + $1; }", ["iii"], [""]]
"33": ["{ return $0 + $1; }", ["iii"], [""]]
},
"staticBump": 0,
"tableSize": 0,
Expand Down