Skip to content

Commit fb78a12

Browse files
authored
Use absolute memory addresses for emasm string indexes. (#2408)
Before we used 0-based indexes which meant that we needed to modify the code calling the emasm function to replace the first argument. Now we use the string address itself as the index/code for the emasm constant. This allows use code to go unmodifed as the emscripten side will accept the memory address as the index/code. See: emscripten-core/emscripten#9013
1 parent 2a787cd commit fb78a12

6 files changed

+76
-54
lines changed

src/wasm/wasm-emscripten.cpp

+36-25
Original file line numberDiff line numberDiff line change
@@ -653,8 +653,7 @@ const char* stringAtAddr(Module& wasm,
653653

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

711710
private:
712711
std::string fixupName(Name& name, std::string baseSig, Proxying proxy);
713-
AsmConst& createAsmConst(std::string code, std::string sig, Name name);
712+
AsmConst&
713+
createAsmConst(uint32_t id, std::string code, std::string sig, Name name);
714714
std::string asmConstSig(std::string baseSig);
715715
Name nameForImportWithSig(std::string sig, Proxying proxy);
716716
void queueImport(Name importName, std::string baseSig);
@@ -757,29 +757,38 @@ void AsmConstWalker::visitCall(Call* curr) {
757757
<< ".\nThis might be caused by aggressive compiler "
758758
"transformations. Consider using EM_JS instead.";
759759
}
760-
} else if (auto* value = arg->dynCast<Binary>()) {
761-
// In the dynamic linking case the address of the string constant
762-
// is the result of adding its offset to __memory_base.
763-
// In this case are only looking for the offset with the data segment so
764-
// the RHS of the addition is just what we want.
765-
assert(value->op == AddInt32);
766-
arg = value->right;
767-
} else {
768-
if (!value) {
769-
Fatal() << "Unexpected arg0 type (" << getExpressionName(arg)
770-
<< ") in call to: " << importName;
760+
continue;
761+
}
762+
763+
if (auto* setlocal = arg->dynCast<LocalSet>()) {
764+
// The argument may be a local.tee, in which case we take first child
765+
// which is the value being copied into the local.
766+
if (setlocal->isTee()) {
767+
arg = setlocal->value;
768+
continue;
771769
}
772770
}
771+
772+
if (auto* bin = arg->dynCast<Binary>()) {
773+
if (bin->op == AddInt32) {
774+
// In the dynamic linking case the address of the string constant
775+
// is the result of adding its offset to __memory_base.
776+
// In this case are only looking for the offset from __memory_base
777+
// the RHS of the addition is just what we want.
778+
arg = bin->right;
779+
continue;
780+
}
781+
}
782+
783+
Fatal() << "Unexpected arg0 type (" << getExpressionName(arg)
784+
<< ") in call to: " << importName;
773785
}
774786

775787
auto* value = arg->cast<Const>();
776-
auto code = codeForConstAddr(wasm, segmentOffsets, value);
777-
auto& asmConst = createAsmConst(code, sig, importName);
788+
int32_t address = value->value.geti32();
789+
auto code = codeForConstAddr(wasm, segmentOffsets, address);
790+
auto& asmConst = createAsmConst(address, code, sig, importName);
778791
fixupName(curr->target, baseSig, asmConst.proxy);
779-
780-
// Replace the first argument to the call with a Const index
781-
Builder builder(wasm);
782-
curr->operands[0] = builder.makeConst(Literal(asmConst.id));
783792
}
784793

785794
Proxying AsmConstWalker::proxyType(Name name) {
@@ -826,14 +835,15 @@ AsmConstWalker::fixupName(Name& name, std::string baseSig, Proxying proxy) {
826835
return sig;
827836
}
828837

829-
AsmConstWalker::AsmConst&
830-
AsmConstWalker::createAsmConst(std::string code, std::string sig, Name name) {
838+
AsmConstWalker::AsmConst& AsmConstWalker::createAsmConst(uint32_t id,
839+
std::string code,
840+
std::string sig,
841+
Name name) {
831842
if (asmConsts.count(code) == 0) {
832843
AsmConst asmConst;
833-
asmConst.id = asmConsts.size();
844+
asmConst.id = id;
834845
asmConst.sigs.insert(sig);
835846
asmConst.proxy = proxyType(name);
836-
837847
asmConsts[code] = asmConst;
838848
}
839849
return asmConsts[code];
@@ -918,7 +928,8 @@ struct EmJsWalker : public PostWalker<EmJsWalker> {
918928
Fatal() << "Unexpected generated __em_js__ function body: " << curr->name;
919929
}
920930
auto* addrConst = consts.list[0];
921-
auto code = codeForConstAddr(wasm, segmentOffsets, addrConst);
931+
int32_t address = addrConst->value.geti32();
932+
auto code = codeForConstAddr(wasm, segmentOffsets, address);
922933
codeByName[funcName] = code;
923934
}
924935
};

test/lld/em_asm.wast.mem.out

+6-6
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
)
4343
(drop
4444
(call $emscripten_asm_const_iii
45-
(i32.const 0)
45+
(i32.const 568)
4646
(i32.add
4747
(local.get $0)
4848
(i32.const 24)
@@ -69,7 +69,7 @@
6969
)
7070
(local.tee $1
7171
(call $emscripten_asm_const_iii
72-
(i32.const 1)
72+
(i32.const 601)
7373
(i32.add
7474
(local.get $0)
7575
(i32.const 24)
@@ -87,7 +87,7 @@
8787
)
8888
(drop
8989
(call $emscripten_asm_const_iii
90-
(i32.const 2)
90+
(i32.const 621)
9191
(i32.add
9292
(local.get $0)
9393
(i32.const 24)
@@ -226,9 +226,9 @@
226226
--BEGIN METADATA --
227227
{
228228
"asmConsts": {
229-
"2": ["{ Module.print(\"Got \" + $0); }", ["iii"], [""]],
230-
"0": ["{ Module.print(\"Hello world\"); }", ["iii"], [""]],
231-
"1": ["{ return $0 + $1; }", ["iii"], [""]]
229+
"621": ["{ Module.print(\"Got \" + $0); }", ["iii"], [""]],
230+
"568": ["{ Module.print(\"Hello world\"); }", ["iii"], [""]],
231+
"601": ["{ return $0 + $1; }", ["iii"], [""]]
232232
},
233233
"staticBump": 84,
234234
"tableSize": 1,

test/lld/em_asm.wast.out

+6-6
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
)
4444
(drop
4545
(call $emscripten_asm_const_iii
46-
(i32.const 0)
46+
(i32.const 568)
4747
(i32.add
4848
(local.get $0)
4949
(i32.const 24)
@@ -70,7 +70,7 @@
7070
)
7171
(local.tee $1
7272
(call $emscripten_asm_const_iii
73-
(i32.const 1)
73+
(i32.const 601)
7474
(i32.add
7575
(local.get $0)
7676
(i32.const 24)
@@ -88,7 +88,7 @@
8888
)
8989
(drop
9090
(call $emscripten_asm_const_iii
91-
(i32.const 2)
91+
(i32.const 621)
9292
(i32.add
9393
(local.get $0)
9494
(i32.const 24)
@@ -227,9 +227,9 @@
227227
--BEGIN METADATA --
228228
{
229229
"asmConsts": {
230-
"2": ["{ Module.print(\"Got \" + $0); }", ["iii"], [""]],
231-
"0": ["{ Module.print(\"Hello world\"); }", ["iii"], [""]],
232-
"1": ["{ return $0 + $1; }", ["iii"], [""]]
230+
"621": ["{ Module.print(\"Got \" + $0); }", ["iii"], [""]],
231+
"568": ["{ Module.print(\"Hello world\"); }", ["iii"], [""]],
232+
"601": ["{ return $0 + $1; }", ["iii"], [""]]
233233
},
234234
"staticBump": 84,
235235
"tableSize": 1,

test/lld/em_asm_O0.wast.out

+6-6
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
(local $t2 i32)
3131
(drop
3232
(call $emscripten_asm_const_i
33-
(i32.const 0)
33+
(i32.const 568)
3434
)
3535
)
3636
(local.set $t1
@@ -41,9 +41,9 @@
4141
)
4242
(drop
4343
(call $emscripten_asm_const_ii
44-
(i32.const 2)
44+
(local.get $t1)
4545
(call $emscripten_asm_const_iii
46-
(i32.const 1)
46+
(local.get $t2)
4747
(i32.const 13)
4848
(i32.const 27)
4949
)
@@ -87,9 +87,9 @@
8787
--BEGIN METADATA --
8888
{
8989
"asmConsts": {
90-
"2": ["{ Module.print(\"Got \" + $0); }", ["ii"], [""]],
91-
"0": ["{ Module.print(\"Hello world\"); }", ["i"], [""]],
92-
"1": ["{ return $0 + $1; }", ["iii"], [""]]
90+
"621": ["{ Module.print(\"Got \" + $0); }", ["ii"], [""]],
91+
"568": ["{ Module.print(\"Hello world\"); }", ["i"], [""]],
92+
"601": ["{ return $0 + $1; }", ["iii"], [""]]
9393
},
9494
"staticBump": 84,
9595
"tableSize": 1,

test/lld/em_asm_main_thread.wast.out

+6-6
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
)
4444
(drop
4545
(call $emscripten_asm_const_sync_on_main_thread_iii
46-
(i32.const 0)
46+
(i32.const 568)
4747
(i32.add
4848
(local.get $0)
4949
(i32.const 24)
@@ -70,7 +70,7 @@
7070
)
7171
(local.tee $1
7272
(call $emscripten_asm_const_sync_on_main_thread_iii
73-
(i32.const 1)
73+
(i32.const 601)
7474
(i32.add
7575
(local.get $0)
7676
(i32.const 24)
@@ -88,7 +88,7 @@
8888
)
8989
(drop
9090
(call $emscripten_asm_const_sync_on_main_thread_iii
91-
(i32.const 2)
91+
(i32.const 621)
9292
(i32.add
9393
(local.get $0)
9494
(i32.const 24)
@@ -227,9 +227,9 @@
227227
--BEGIN METADATA --
228228
{
229229
"asmConsts": {
230-
"2": ["{ Module.print(\"Got \" + $0); }", ["iii"], ["sync_on_main_thread_"]],
231-
"0": ["{ Module.print(\"Hello world\"); }", ["iii"], ["sync_on_main_thread_"]],
232-
"1": ["{ return $0 + $1; }", ["iii"], ["sync_on_main_thread_"]]
230+
"621": ["{ Module.print(\"Got \" + $0); }", ["iii"], ["sync_on_main_thread_"]],
231+
"568": ["{ Module.print(\"Hello world\"); }", ["iii"], ["sync_on_main_thread_"]],
232+
"601": ["{ return $0 + $1; }", ["iii"], ["sync_on_main_thread_"]]
233233
},
234234
"staticBump": 84,
235235
"tableSize": 1,

test/lld/em_asm_shared.wast.out

+16-5
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,12 @@
4848
)
4949
(drop
5050
(call $emscripten_asm_const_iii
51-
(i32.const 0)
51+
(i32.add
52+
(local.tee $1
53+
(global.get $gimport$3)
54+
)
55+
(i32.const 0)
56+
)
5257
(i32.add
5358
(local.get $0)
5459
(i32.const 24)
@@ -75,7 +80,10 @@
7580
)
7681
(local.tee $2
7782
(call $emscripten_asm_const_iii
78-
(i32.const 1)
83+
(i32.add
84+
(local.get $1)
85+
(i32.const 33)
86+
)
7987
(i32.add
8088
(local.get $0)
8189
(i32.const 24)
@@ -93,7 +101,10 @@
93101
)
94102
(drop
95103
(call $emscripten_asm_const_iii
96-
(i32.const 2)
104+
(i32.add
105+
(local.get $1)
106+
(i32.const 53)
107+
)
97108
(i32.add
98109
(local.get $0)
99110
(i32.const 24)
@@ -207,9 +218,9 @@
207218
--BEGIN METADATA --
208219
{
209220
"asmConsts": {
210-
"2": ["{ Module.print(\"Got \" + $0); }", ["iii"], [""]],
221+
"53": ["{ Module.print(\"Got \" + $0); }", ["iii"], [""]],
211222
"0": ["{ Module.print(\"Hello world\"); }", ["iii"], [""]],
212-
"1": ["{ return $0 + $1; }", ["iii"], [""]]
223+
"33": ["{ return $0 + $1; }", ["iii"], [""]]
213224
},
214225
"staticBump": 0,
215226
"tableSize": 0,

0 commit comments

Comments
 (0)