Skip to content

Commit 3b61729

Browse files
sbc100sbc100
authored
Remvoe SAFE_HEAP handling makeGet/SetValue. NFC (#17040)
Converting heap accessed to `SAFE_HEAP` is these days handled by a pass in `tools/acorn-optimizer.js` so the use of `SAFE_HEAP` here was redundant.
1 parent eb09362 commit 3b61729

File tree

7 files changed

+69
-74
lines changed

7 files changed

+69
-74
lines changed

‎emcc.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1825,7 +1825,9 @@ def phase_linker_setup(options, state, newargs, user_settings):
18251825
if not settings.BOOTSTRAPPING_STRUCT_INFO:
18261826
# Include the internal library function since they are used by runtime functions.
18271827
settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['$getWasmTableEntry', '$setWasmTableEntry']
1828-
if settings.SAFE_HEAP or not settings.MINIMAL_RUNTIME:
1828+
if settings.SAFE_HEAP:
1829+
settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['$getValue_safe', '$setValue_safe']
1830+
if not settings.MINIMAL_RUNTIME:
18291831
settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['$getValue', '$setValue']
18301832

18311833
if settings.MAIN_MODULE:

‎src/library_getvalue.js‎

Lines changed: 43 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -6,27 +6,13 @@
66

77
var LibraryMemOps = {
88
$setValue__docs: `
9-
/** @param {number} ptr
10-
@param {number} value
11-
@param {string} type
12-
@param {number|boolean=} noSafe */`,
13-
$setValue: function(ptr, value, type = 'i8', noSafe) {
9+
/**
10+
* @param {number} ptr
11+
* @param {number} value
12+
* @param {string} type
13+
*/`,
14+
$setValue: function(ptr, value, type = 'i8') {
1415
if (type.endsWith('*')) type = '{{{ POINTER_WASM_TYPE }}}';
15-
#if SAFE_HEAP
16-
if (noSafe) {
17-
switch (type) {
18-
case 'i1': {{{ makeSetValue('ptr', '0', 'value', 'i1', undefined, undefined, undefined, /*noSafe=*/true) }}}; break;
19-
case 'i8': {{{ makeSetValue('ptr', '0', 'value', 'i8', undefined, undefined, undefined, /*noSafe=*/true) }}}; break;
20-
case 'i16': {{{ makeSetValue('ptr', '0', 'value', 'i16', undefined, undefined, undefined, /*noSafe=*/true) }}}; break;
21-
case 'i32': {{{ makeSetValue('ptr', '0', 'value', 'i32', undefined, undefined, undefined, /*noSafe=*/true) }}}; break;
22-
case 'i64': {{{ makeSetValue('ptr', '0', 'value', 'i64', undefined, undefined, undefined, /*noSafe=*/true) }}}; break;
23-
case 'float': {{{ makeSetValue('ptr', '0', 'value', 'float', undefined, undefined, undefined, /*noSafe=*/true) }}}; break;
24-
case 'double': {{{ makeSetValue('ptr', '0', 'value', 'double', undefined, undefined, undefined, /*noSafe=*/true) }}}; break;
25-
default: abort('invalid type for setValue: ' + type);
26-
}
27-
return;
28-
}
29-
#endif
3016
switch (type) {
3117
case 'i1': {{{ makeSetValue('ptr', '0', 'value', 'i1') }}}; break;
3218
case 'i8': {{{ makeSetValue('ptr', '0', 'value', 'i8') }}}; break;
@@ -40,26 +26,45 @@ var LibraryMemOps = {
4026
},
4127

4228
$getValue__docs: `
43-
/** @param {number} ptr
44-
@param {string} type
45-
@param {number|boolean=} noSafe */`,
46-
$getValue: function(ptr, type = 'i8', noSafe) {
29+
/**
30+
* @param {number} ptr
31+
* @param {string} type
32+
*/`,
33+
$getValue: function(ptr, type = 'i8') {
4734
if (type.endsWith('*')) type = '{{{ POINTER_WASM_TYPE }}}';
35+
switch (type) {
36+
case 'i1': return {{{ makeGetValue('ptr', '0', 'i1') }}};
37+
case 'i8': return {{{ makeGetValue('ptr', '0', 'i8') }}};
38+
case 'i16': return {{{ makeGetValue('ptr', '0', 'i16') }}};
39+
case 'i32': return {{{ makeGetValue('ptr', '0', 'i32') }}};
40+
case 'i64': return {{{ makeGetValue('ptr', '0', 'i64') }}};
41+
case 'float': return {{{ makeGetValue('ptr', '0', 'float') }}};
42+
case 'double': return {{{ makeGetValue('ptr', '0', 'double') }}};
43+
default: abort('invalid type for getValue: ' + type);
44+
}
45+
return null;
46+
},
47+
4848
#if SAFE_HEAP
49-
if (noSafe) {
50-
switch (type) {
51-
case 'i1': return {{{ makeGetValue('ptr', '0', 'i1', undefined, undefined, undefined, undefined, /*noSafe=*/true) }}};
52-
case 'i8': return {{{ makeGetValue('ptr', '0', 'i8', undefined, undefined, undefined, undefined, /*noSafe=*/true) }}};
53-
case 'i16': return {{{ makeGetValue('ptr', '0', 'i16', undefined, undefined, undefined, undefined, /*noSafe=*/true) }}};
54-
case 'i32': return {{{ makeGetValue('ptr', '0', 'i32', undefined, undefined, undefined, undefined, /*noSafe=*/true) }}};
55-
case 'i64': return {{{ makeGetValue('ptr', '0', 'i64', undefined, undefined, undefined, undefined, /*noSafe=*/true) }}};
56-
case 'float': return {{{ makeGetValue('ptr', '0', 'float', undefined, undefined, undefined, undefined, /*noSafe=*/true) }}};
57-
case 'double': return {{{ makeGetValue('ptr', '0', 'double', undefined, undefined, undefined, undefined, /*noSafe=*/true) }}};
58-
default: abort('invalid type for getValue: ' + type);
59-
}
60-
return;
49+
// Identical to above two functions, but known to the safeHeap pass
50+
// in tools/acorn-optimizer.js. The heap accesses here in these two
51+
// functions will not get re-written.
52+
$setValue_safe__internal: true,
53+
$setValue_safe: function(ptr, value, type) {
54+
switch (type) {
55+
case 'i1': {{{ makeSetValue('ptr', '0', 'value', 'i1') }}}; break;
56+
case 'i8': {{{ makeSetValue('ptr', '0', 'value', 'i8') }}}; break;
57+
case 'i16': {{{ makeSetValue('ptr', '0', 'value', 'i16') }}}; break;
58+
case 'i32': {{{ makeSetValue('ptr', '0', 'value', 'i32') }}}; break;
59+
case 'i64': {{{ makeSetValue('ptr', '0', 'value', 'i64') }}}; break;
60+
case 'float': {{{ makeSetValue('ptr', '0', 'value', 'float') }}}; break;
61+
case 'double': {{{ makeSetValue('ptr', '0', 'value', 'double') }}}; break;
62+
default: abort('invalid type for setValue: ' + type);
6163
}
62-
#endif
64+
},
65+
66+
$getValue_safe__internal: true,
67+
$getValue_safe: function(ptr, type) {
6368
switch (type) {
6469
case 'i1': return {{{ makeGetValue('ptr', '0', 'i1') }}};
6570
case 'i8': return {{{ makeGetValue('ptr', '0', 'i8') }}};
@@ -70,8 +75,8 @@ var LibraryMemOps = {
7075
case 'double': return {{{ makeGetValue('ptr', '0', 'double') }}};
7176
default: abort('invalid type for getValue: ' + type);
7277
}
73-
return null;
7478
},
79+
#endif
7580
};
7681

7782
mergeInto(LibraryManager.library, LibraryMemOps);

‎src/parseTools.js‎

Lines changed: 15 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ function makeSetTempDouble(i, type, value) {
368368
}
369369

370370
// See makeSetValue
371-
function makeGetValue(ptr, pos, type, noNeedFirst, unsigned, ignore, align, noSafe) {
371+
function makeGetValue(ptr, pos, type, noNeedFirst, unsigned, ignore, align) {
372372
if (typeof unsigned !== 'undefined') {
373373
// TODO(sbc): make this into an error at some point.
374374
printErr('makeGetValue: Please use u8/u16/u32/u64 unsigned types in favor of additional argument');
@@ -381,8 +381,8 @@ function makeGetValue(ptr, pos, type, noNeedFirst, unsigned, ignore, align, noSa
381381
}
382382

383383
if (type == 'double' && (align < 8)) {
384-
const setdouble1 = makeSetTempDouble(0, 'i32', makeGetValue(ptr, pos, 'i32', noNeedFirst, unsigned, ignore, align, noSafe));
385-
const setdouble2 = makeSetTempDouble(1, 'i32', makeGetValue(ptr, getFastValue(pos, '+', Runtime.getNativeTypeSize('i32')), 'i32', noNeedFirst, unsigned, ignore, align, noSafe));
384+
const setdouble1 = makeSetTempDouble(0, 'i32', makeGetValue(ptr, pos, 'i32', noNeedFirst, unsigned, ignore, align));
385+
const setdouble2 = makeSetTempDouble(1, 'i32', makeGetValue(ptr, getFastValue(pos, '+', Runtime.getNativeTypeSize('i32')), 'i32', noNeedFirst, unsigned, ignore, align));
386386
return '(' + setdouble1 + ',' + setdouble2 + ',' + makeGetTempDouble(0, 'double') + ')';
387387
}
388388

@@ -394,12 +394,12 @@ function makeGetValue(ptr, pos, type, noNeedFirst, unsigned, ignore, align, noSa
394394
if (isIntImplemented(type)) {
395395
if (bytes == 4 && align == 2) {
396396
// Special case that we can optimize
397-
ret += makeGetValue(ptr, pos, 'i16', noNeedFirst, 2, ignore, 2, noSafe) + '|' +
398-
'(' + makeGetValue(ptr, getFastValue(pos, '+', 2), 'i16', noNeedFirst, 2, ignore, 2, noSafe) + '<<16)';
397+
ret += makeGetValue(ptr, pos, 'i16', noNeedFirst, 2, ignore, 2) + '|' +
398+
'(' + makeGetValue(ptr, getFastValue(pos, '+', 2), 'i16', noNeedFirst, 2, ignore, 2) + '<<16)';
399399
} else { // XXX we cannot truly handle > 4... (in x86)
400400
ret = '';
401401
for (let i = 0; i < bytes; i++) {
402-
ret += '(' + makeGetValue(ptr, getFastValue(pos, '+', i), 'i8', noNeedFirst, 1, ignore, 1, noSafe) + (i > 0 ? '<<' + (8 * i) : '') + ')';
402+
ret += '(' + makeGetValue(ptr, getFastValue(pos, '+', i), 'i8', noNeedFirst, 1, ignore, 1) + (i > 0 ? '<<' + (8 * i) : '') + ')';
403403
if (i < bytes - 1) ret += '|';
404404
}
405405
ret = '(' + makeSignOp(ret, type, unsigned ? 'un' : 're', true);
@@ -417,11 +417,6 @@ function makeGetValue(ptr, pos, type, noNeedFirst, unsigned, ignore, align, noSa
417417
}
418418

419419
const offset = calcFastOffset(ptr, pos, noNeedFirst);
420-
if (SAFE_HEAP && !noSafe) {
421-
if (!ignore) {
422-
return asmCoercion('SAFE_HEAP_LOAD' + (FLOAT_TYPES.has(type) ? '_D' : '') + '(' + asmCoercion(offset, 'i32') + ', ' + Runtime.getNativeTypeSize(type) + ', ' + (!!unsigned + 0) + ')', type, unsigned ? 'u' : undefined);
423-
}
424-
}
425420

426421
const slab = getHeapForType(type);
427422
let ret = slab + '[' + getHeapOffset(offset, type) + ']';
@@ -439,24 +434,22 @@ function makeGetValue(ptr, pos, type, noNeedFirst, unsigned, ignore, align, noSa
439434
* @param {nunber} pos The position in that slab - the offset. Added to any offset in the pointer itself.
440435
* @param {number} value The value to set.
441436
* @param {string} type A string defining the type. Used to find the slab (HEAPU8, HEAP16, HEAPU32, etc.).
442-
* 'null' means, in the context of SAFE_HEAP, that we should accept all types;
443437
* which means we should write to all slabs, ignore type differences if any on reads, etc.
444438
* @param {bool} noNeedFirst Whether to ignore the offset in the pointer itself.
445439
* @param {bool} ignore: legacy, ignored.
446440
* @param {number} align: TODO
447-
* @param {bool} noSafe: TODO
448441
* @param {string} sep: TODO
449442
* @return {TODO}
450443
*/
451-
function makeSetValue(ptr, pos, value, type, noNeedFirst, ignore, align, noSafe, sep = ';') {
444+
function makeSetValue(ptr, pos, value, type, noNeedFirst, ignore, align, sep = ';') {
452445
if (type == 'double' && (align < 8)) {
453446
return '(' + makeSetTempDouble(0, 'double', value) + ',' +
454-
makeSetValue(ptr, pos, makeGetTempDouble(0, 'i32'), 'i32', noNeedFirst, ignore, align, noSafe, ',') + ',' +
455-
makeSetValue(ptr, getFastValue(pos, '+', Runtime.getNativeTypeSize('i32')), makeGetTempDouble(1, 'i32'), 'i32', noNeedFirst, ignore, align, noSafe, ',') + ')';
447+
makeSetValue(ptr, pos, makeGetTempDouble(0, 'i32'), 'i32', noNeedFirst, ignore, align, ',') + ',' +
448+
makeSetValue(ptr, getFastValue(pos, '+', Runtime.getNativeTypeSize('i32')), makeGetTempDouble(1, 'i32'), 'i32', noNeedFirst, ignore, align, ',') + ')';
456449
} else if (!WASM_BIGINT && type == 'i64') {
457450
return '(tempI64 = [' + splitI64(value) + '],' +
458-
makeSetValue(ptr, pos, 'tempI64[0]', 'i32', noNeedFirst, ignore, align, noSafe, ',') + ',' +
459-
makeSetValue(ptr, getFastValue(pos, '+', Runtime.getNativeTypeSize('i32')), 'tempI64[1]', 'i32', noNeedFirst, ignore, align, noSafe, ',') + ')';
451+
makeSetValue(ptr, pos, 'tempI64[0]', 'i32', noNeedFirst, ignore, align, ',') + ',' +
452+
makeSetValue(ptr, getFastValue(pos, '+', Runtime.getNativeTypeSize('i32')), 'tempI64[1]', 'i32', noNeedFirst, ignore, align, ',') + ')';
460453
}
461454

462455
const bits = getBits(type);
@@ -470,29 +463,24 @@ function makeSetValue(ptr, pos, value, type, noNeedFirst, ignore, align, noSafe,
470463
if (bytes == 4 && align == 2) {
471464
// Special case that we can optimize
472465
ret += 'tempBigInt=' + value + sep;
473-
ret += makeSetValue(ptr, pos, 'tempBigInt&0xffff', 'i16', noNeedFirst, ignore, 2, noSafe) + sep;
474-
ret += makeSetValue(ptr, getFastValue(pos, '+', 2), 'tempBigInt>>16', 'i16', noNeedFirst, ignore, 2, noSafe);
466+
ret += makeSetValue(ptr, pos, 'tempBigInt&0xffff', 'i16', noNeedFirst, ignore, 2) + sep;
467+
ret += makeSetValue(ptr, getFastValue(pos, '+', 2), 'tempBigInt>>16', 'i16', noNeedFirst, ignore, 2);
475468
} else {
476469
ret += 'tempBigInt=' + value + sep;
477470
for (let i = 0; i < bytes; i++) {
478-
ret += makeSetValue(ptr, getFastValue(pos, '+', i), 'tempBigInt&0xff', 'i8', noNeedFirst, ignore, 1, noSafe);
471+
ret += makeSetValue(ptr, getFastValue(pos, '+', i), 'tempBigInt&0xff', 'i8', noNeedFirst, ignore, 1);
479472
if (i < bytes - 1) ret += sep + 'tempBigInt = tempBigInt>>8' + sep;
480473
}
481474
}
482475
} else {
483-
ret += makeSetValue('tempDoublePtr', 0, value, type, noNeedFirst, ignore, 8, noSafe, null, true) + sep;
476+
ret += makeSetValue('tempDoublePtr', 0, value, type, noNeedFirst, ignore, 8) + sep;
484477
ret += makeCopyValues(getFastValue(ptr, '+', pos), 'tempDoublePtr', Runtime.getNativeTypeSize(type), type, null, align, sep);
485478
}
486479
return ret;
487480
}
488481
}
489482

490483
const offset = calcFastOffset(ptr, pos, noNeedFirst);
491-
if (SAFE_HEAP && !noSafe) {
492-
if (!ignore) {
493-
return 'SAFE_HEAP_STORE' + (FLOAT_TYPES.has(type) ? '_D' : '') + '(' + asmCoercion(offset, 'i32') + ', ' + asmCoercion(value, type) + ', ' + Runtime.getNativeTypeSize(type) + ')';
494-
}
495-
}
496484

497485
const slab = getHeapForType(type);
498486
if (slab == 'HEAPU64' || slab == 'HEAP64') {

‎src/runtime_safe_heap.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ function SAFE_HEAP_STORE(dest, value, bytes, isFloat) {
4747
assert(brk >= _emscripten_stack_get_base()); // sbrk-managed memory must be above the stack
4848
assert(brk <= HEAP8.length);
4949
}
50-
setValue(dest, value, getSafeHeapType(bytes, isFloat), 1);
50+
setValue_safe(dest, value, getSafeHeapType(bytes, isFloat));
5151
return value;
5252
}
5353
function SAFE_HEAP_STORE_D(dest, value, bytes) {
@@ -76,7 +76,7 @@ function SAFE_HEAP_LOAD(dest, bytes, unsigned, isFloat) {
7676
assert(brk <= HEAP8.length);
7777
}
7878
var type = getSafeHeapType(bytes, isFloat);
79-
var ret = getValue(dest, type, 1);
79+
var ret = getValue_safe(dest, type);
8080
if (unsigned) ret = unSign(ret, parseInt(type.substr(1), 10));
8181
#if SAFE_HEAP_LOG
8282
out('SAFE_HEAP load: ' + [dest, ret, bytes, isFloat, unsigned, SAFE_HEAP_COUNTER++]);

‎tests/optimizer/test-safeHeap-output.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,11 @@ function SAFE_HEAP_FOO(ptr) {
4040
return HEAP8[ptr];
4141
}
4242

43-
function setValue(ptr) {
43+
function setValue_safe(ptr) {
4444
return HEAP8[ptr];
4545
}
4646

47-
function getValue(ptr) {
47+
function getValue_safe(ptr) {
4848
return HEAP8[ptr];
4949
}
5050

‎tests/optimizer/test-safeHeap.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,10 @@ HEAPF32[x] = HEAP32[y];
3636
function SAFE_HEAP_FOO(ptr) {
3737
return HEAP8[ptr];
3838
}
39-
function setValue(ptr) {
39+
function setValue_safe(ptr) {
4040
return HEAP8[ptr];
4141
}
42-
function getValue(ptr) {
42+
function getValue_safe(ptr) {
4343
return HEAP8[ptr];
4444
}
4545

‎tools/acorn-optimizer.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1374,8 +1374,8 @@ function safeHeap(ast) {
13741374
FunctionDeclaration(node, c) {
13751375
if (node.id.type === 'Identifier' &&
13761376
(node.id.name.startsWith('SAFE_HEAP') ||
1377-
node.id.name === 'setValue' ||
1378-
node.id.name === 'getValue')) {
1377+
node.id.name === 'setValue_safe' ||
1378+
node.id.name === 'getValue_safe')) {
13791379
// do not recurse into this js impl function, which we use during
13801380
// startup before the wasm is ready
13811381
} else {

0 commit comments

Comments
 (0)