Skip to content

Commit f8a38f7

Browse files
CelluleCellule
committed
[MERGE chakra-core#219] Memset on typed specialized values
Merge pull request chakra-core#219 from Cellule:users/micfer/memop_var Sometimes, when type specializing the original instruction is lost. ie: `s3.var = LdC_A_I4 0 (0x0).i32` is type specialized to `int32`, but the dst is used as a float. If it's the only use, then `s3.var` is removed. Previously memset was trying to get `s3.var` from the float type specialized version. However, it is not live anymore. This change makes memset use directly the value the `StElemI_A` is using (ie: the float typed specialized register) And will call ToVar on it. A better fix would be to try to use the var version first if it is available, but might introduce unexpected bugs. I also added several test cases to cover this issue and more, along with some tests with simds.
2 parents 94b77f8 + e08929f commit f8a38f7

8 files changed

Lines changed: 225 additions & 131 deletions

File tree

‎lib/Backend/FlowGraph.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -619,9 +619,9 @@ class Loop
619619
struct MemSetCandidate : public MemOpCandidate
620620
{
621621
BailoutConstantValue constant;
622-
StackSym* varSym;
622+
StackSym* srcSym;
623623

624-
MemSetCandidate() : MemOpCandidate(MemOpCandidate::MEMSET), varSym(nullptr) {}
624+
MemSetCandidate() : MemOpCandidate(MemOpCandidate::MEMSET), srcSym(nullptr) {}
625625
};
626626

627627
struct MemCopyCandidate : public MemOpCandidate

‎lib/Backend/GlobOpt.cpp‎

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4248,23 +4248,14 @@ GlobOpt::CollectMemsetStElementI(IR::Instr *instr, Loop *loop)
42484248
SymID baseSymID = GetVarSymID(baseOp->GetStackSym());
42494249

42504250
IR::Opnd *srcDef = instr->GetSrc1();
4251-
StackSym *varSym = nullptr;
4251+
StackSym *srcSym = nullptr;
42524252
if (srcDef->IsRegOpnd())
42534253
{
42544254
IR::RegOpnd* opnd = srcDef->AsRegOpnd();
42554255
if (this->OptIsInvariant(opnd, this->currentBlock, loop, this->FindValue(opnd->m_sym), true, true))
42564256
{
4257-
StackSym* sym = opnd->GetStackSym();
4258-
if (sym->GetType() != TyVar)
4259-
{
4260-
varSym = sym->GetVarEquivSym(instr->m_func);
4261-
}
4262-
else
4263-
{
4264-
varSym = sym;
4265-
}
4257+
srcSym = opnd->GetStackSym();
42664258
}
4267-
42684259
}
42694260

42704261
BailoutConstantValue constant = {TyIllegal, 0};
@@ -4280,7 +4271,7 @@ GlobOpt::CollectMemsetStElementI(IR::Instr *instr, Loop *loop)
42804271
{
42814272
constant.InitVarConstValue(srcDef->AsAddrOpnd()->m_address);
42824273
}
4283-
else if(!varSym)
4274+
else if(!srcSym)
42844275
{
42854276
TRACE_MEMOP_PHASE_VERBOSE(MemSet, loop, instr, L"Source is not an invariant");
42864277
return false;
@@ -4297,7 +4288,7 @@ GlobOpt::CollectMemsetStElementI(IR::Instr *instr, Loop *loop)
42974288
memsetInfo->base = baseSymID;
42984289
memsetInfo->index = inductionSymID;
42994290
memsetInfo->constant = constant;
4300-
memsetInfo->varSym = varSym;
4291+
memsetInfo->srcSym = srcSym;
43014292
memsetInfo->count = 1;
43024293
memsetInfo->bIndexAlreadyChanged = isIndexPreIncr;
43034294
loop->memOpInfo->candidates->Prepend(memsetInfo);
@@ -20821,10 +20812,9 @@ GlobOpt::EmitMemop(Loop * loop, LoopCount *loopCount, const MemOpEmitData* emitD
2082120812
{
2082220813
MemSetEmitData* data = (MemSetEmitData*)emitData;
2082320814
const Loop::MemSetCandidate* candidate = data->candidate->AsMemSet();
20824-
if (candidate->varSym)
20815+
if (candidate->srcSym)
2082520816
{
20826-
Assert(candidate->varSym->GetType() == TyVar);
20827-
IR::RegOpnd* regSrc = IR::RegOpnd::New(candidate->varSym, TyVar, func);
20817+
IR::RegOpnd* regSrc = IR::RegOpnd::New(candidate->srcSym, candidate->srcSym->GetType(), func);
2082820818
regSrc->SetIsJITOptimizedReg(true);
2082920819
src1 = regSrc;
2083020820
}
@@ -20877,9 +20867,9 @@ GlobOpt::EmitMemop(Loop * loop, LoopCount *loopCount, const MemOpEmitData* emitD
2087720867
const Loop::MemSetCandidate* candidate = emitData->candidate->AsMemSet();
2087820868
const int constBufSize = 32;
2087920869
wchar_t constBuf[constBufSize];
20880-
if (candidate->varSym)
20870+
if (candidate->srcSym)
2088120871
{
20882-
_snwprintf_s(constBuf, constBufSize, L"s%u", candidate->varSym->m_id);
20872+
_snwprintf_s(constBuf, constBufSize, L"s%u", candidate->srcSym->m_id);
2088320873
}
2088420874
else
2088520875
{

‎lib/Backend/Lower.cpp‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1508,7 +1508,7 @@ Lowerer::LowerRange(IR::Instr *instrStart, IR::Instr *instrEnd, bool defaultDoFa
15081508
case Js::OpCode::Memset:
15091509
case Js::OpCode::Memcopy:
15101510
{
1511-
LowerMemOp(instr);
1511+
instrPrev = LowerMemOp(instr);
15121512
break;
15131513
}
15141514

@@ -8379,7 +8379,7 @@ Lowerer::LowerLdArrViewElem(IR::Instr * instr)
83798379
return instrPrev;
83808380
}
83818381

8382-
void
8382+
IR::Instr *
83838383
Lowerer::LowerMemset(IR::Instr * instr, IR::RegOpnd * helperRet)
83848384
{
83858385
IR::Opnd * dst = instr->UnlinkDst();
@@ -8396,7 +8396,14 @@ Lowerer::LowerMemset(IR::Instr * instr, IR::RegOpnd * helperRet)
83968396
Assert(indexOpnd);
83978397

83988398
IR::JnHelperMethod helperMethod = IR::HelperOp_Memset;
8399-
8399+
IR::Instr *instrPrev = nullptr;
8400+
if (src1->IsRegOpnd() && !src1->IsVar())
8401+
{
8402+
IR::RegOpnd* varOpnd = IR::RegOpnd::New(TyVar, instr->m_func);
8403+
instrPrev = IR::Instr::New(Js::OpCode::ToVar, varOpnd, src1, instr->m_func);
8404+
instr->InsertBefore(instrPrev);
8405+
src1 = varOpnd;
8406+
}
84008407
instr->SetDst(helperRet);
84018408
LoadScriptContext(instr);
84028409
m_lowererMD.LoadHelperArgument(instr, sizeOpnd);
@@ -8405,9 +8412,11 @@ Lowerer::LowerMemset(IR::Instr * instr, IR::RegOpnd * helperRet)
84058412
m_lowererMD.LoadHelperArgument(instr, baseOpnd);
84068413
m_lowererMD.ChangeToHelperCall(instr, helperMethod);
84078414
dst->Free(m_func);
8415+
8416+
return instrPrev;
84088417
}
84098418

8410-
void
8419+
IR::Instr *
84118420
Lowerer::LowerMemcopy(IR::Instr * instr, IR::RegOpnd * helperRet)
84128421
{
84138422
IR::Opnd * dst = instr->UnlinkDst();
@@ -8442,6 +8451,8 @@ Lowerer::LowerMemcopy(IR::Instr * instr, IR::RegOpnd * helperRet)
84428451
m_lowererMD.ChangeToHelperCall(instr, helperMethod);
84438452
dst->Free(m_func);
84448453
src->Free(m_func);
8454+
8455+
return nullptr;
84458456
}
84468457

84478458
IR::Instr *
@@ -8504,13 +8515,19 @@ Lowerer::LowerMemOp(IR::Instr * instr)
85048515
instr->ClearBailOutInfo();
85058516
}
85068517

8518+
IR::Instr* newInstrPrev = nullptr;
85078519
if (instr->m_opcode == Js::OpCode::Memset)
85088520
{
8509-
LowerMemset(instr, helperRet);
8521+
newInstrPrev = LowerMemset(instr, helperRet);
85108522
}
85118523
else if (instr->m_opcode == Js::OpCode::Memcopy)
85128524
{
8513-
LowerMemcopy(instr, helperRet);
8525+
newInstrPrev = LowerMemcopy(instr, helperRet);
8526+
}
8527+
8528+
if (newInstrPrev != nullptr)
8529+
{
8530+
instrPrev = newInstrPrev;
85148531
}
85158532
return instrPrev;
85168533
}

‎lib/Backend/Lower.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,8 @@ class Lowerer
158158
void LowerLdLen(IR::Instr *const instr, const bool isHelper);
159159

160160
IR::Instr * LowerMemOp(IR::Instr * instr);
161-
void LowerMemset(IR::Instr * instr, IR::RegOpnd * helperRet);
162-
void LowerMemcopy(IR::Instr * instr, IR::RegOpnd * helperRet);
161+
IR::Instr * LowerMemset(IR::Instr * instr, IR::RegOpnd * helperRet);
162+
IR::Instr * LowerMemcopy(IR::Instr * instr, IR::RegOpnd * helperRet);
163163

164164
IR::Instr * LowerLdArrViewElem(IR::Instr * instr);
165165
IR::Instr * LowerStArrViewElem(IR::Instr * instr);

‎test/Array/memset_invariant.js‎

Lines changed: 11 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -3,116 +3,24 @@
33
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
44
//-------------------------------------------------------------------------------------------------------
55

6-
function* makeValueGen(a, b) {
7-
// return a for profiling
8-
yield a;
9-
// return b to bailout
10-
yield b;
11-
// return b again to compare with non jit function
12-
yield b;
13-
}
14-
15-
function* makeStartGen(a, b) {
16-
yield 0; // for interpreter
17-
yield 32; // for jitted version
18-
yield 32; // for jitted version
19-
}
20-
21-
function makeTest(name, config) {
22-
const f1 = `function ${name}(arr) {
23-
for(var i = -5; i < 15; ++i) {arr[i] = ${config.v1};}
24-
return arr;
25-
}`;
26-
const f2 = customName => `function ${customName}P(arr, v) {
27-
for(var i = 1; i < 8; ++i) {arr[i] = v;}
28-
return arr;
29-
}`;
30-
const f3 = `function ${name}V(arr) {
31-
const v = ${config.v1};
32-
for(var i = -2; i < 17; ++i) {arr[i] = v;}
33-
return arr;
34-
}`;
35-
const f4 = customName => `function ${customName}Z(arr, start) {
36-
const v = ${config.v1};
37-
for(var i = start; i < 5; ++i) {arr[i] = v;}
38-
return arr;
39-
}`;
40-
41-
const extraTests = (config.wrongTypes || []).map((wrongType, i) => {
42-
const difValue = {f: f2(`${name}W${i}`), compare: f2(`${name}WC${i}`)};
43-
const genValue = makeValueGen(config.v1, wrongType);
44-
Reflect.defineProperty(difValue, "v", {
45-
get: () => genValue.next().value
46-
});
47-
return difValue;
48-
});
49-
50-
const negativeLengthTest = {f: f4(name), compare: f4(`${name}C`), newForCompare: true};
51-
const genIndex = makeStartGen();
52-
Reflect.defineProperty(negativeLengthTest, "v", {
53-
get: () => genIndex.next().value
54-
});
55-
56-
const tests = [
57-
{f: f1},
58-
{f: f2(name), v: config.v2 !== undefined ? config.v2 : config.v1},
59-
{f: f3},
60-
negativeLengthTest
61-
].concat(extraTests);
62-
63-
const convertTest = function(fnText) {
64-
var fn;
65-
eval(`fn = ${fnText}`);
66-
return fn;
67-
};
68-
for(const t of tests) {
69-
t.f = convertTest(t.f);
70-
t.compare = t.compare && convertTest(t.compare);
71-
}
72-
return tests;
73-
}
6+
this.WScript.LoadScriptFile(".\\memset_tester.js");
747

758
const allTypes = [0, 1.5, undefined, null, 9223372036854775807, "string", {a: null, b: "b"}];
9+
7610
const tests = [
77-
{name: "memsetUndefined", v1: undefined, wrongTypes: allTypes},
78-
{name: "memsetNull", v1: null, wrongTypes: allTypes},
79-
{name: "memsetFloat", v1: 3.14, v2: -87.684, wrongTypes: allTypes},
80-
{name: "memsetNumber", v1: 9223372036854775807, v2: -987654987654987, wrongTypes: allTypes},
81-
{name: "memsetBoolean", v1: true, v2: false, wrongTypes: allTypes},
82-
{name: "memsetString", v1: "\"thatString\"", v2: "`A template string`", wrongTypes: allTypes},
83-
{name: "memsetObject", v1: "{test: 1}", v2: [1, 2, 3], wrongTypes: allTypes},
11+
{name: "memsetUndefined", stringValue: undefined},
12+
{name: "memsetNull", stringValue: null},
13+
{name: "memsetInt", stringValue: 0, v2: 1 << 30},
14+
{name: "memsetFloat", stringValue: 3.14, v2: -87.684},
15+
{name: "memsetNumber", stringValue: 9223372036854775807, v2: -987654987654987},
16+
{name: "memsetBoolean", stringValue: true, v2: false},
17+
{name: "memsetString", stringValue: "\"thatString\"", v2: "`A template string`"},
18+
{name: "memsetObject", stringValue: "{test: 1}", v2: [1, 2, 3]},
8419
];
8520

8621
const types = "Int8Array Uint8Array Int16Array Uint16Array Int32Array Uint32Array Float32Array Float64Array Array".split(" ");
87-
const global = this;
8822

89-
let passed = true;
90-
for(const test of tests) {
91-
for(const t of types) {
92-
const fns = makeTest(`${test.name}${t}`, test);
93-
for(const detail of fns) {
94-
const fn = detail.f;
95-
let a1 = fn(new global[t](10), detail.v);
96-
const a2 = fn(new global[t](10), detail.v);
97-
if(detail.compare) {
98-
// the optimized version ran with a different value. Run again with a clean function to compare=
99-
a1 = detail.compare(detail.newForCompare ? new global[t](10) : a1, detail.v);
100-
}
101-
if(a1.length !== a2.length) {
102-
passed = false;
103-
print(`${fn.name} (${t}) didn't return arrays with same length`);
104-
continue;
105-
}
106-
for(let i = 0; i < a1.length; ++i) {
107-
if(a1[i] !== a2[i] && !(isNaN(a1[i]) && isNaN(a2[i]))) {
108-
passed = false;
109-
print(`${fn.name} (${t}): a1[${i}](${a1[i]}) != a2[${i}](${a2[i]})`);
110-
break;
111-
}
112-
}
113-
}
114-
}
115-
}
23+
let passed = RunMemsetTest(tests, types, allTypes);
11624

11725
function memsetSymbol() {const s = Symbol(); const arr = new Array(10); for(let i = 0; i < 10; ++i) {arr[i] = s;} return arr;}
11826
function memsetSymbolV(v) {const arr = new Array(10); for(let i = 0; i < 10; ++i) {arr[i] = v;} return arr;}

‎test/Array/memset_simd.js‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
//-------------------------------------------------------------------------------------------------------
2+
// Copyright (C) Microsoft. All rights reserved.
3+
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
4+
//-------------------------------------------------------------------------------------------------------
5+
6+
this.WScript.LoadScriptFile("memset_tester.js");
7+
8+
const simdRegex = /^\w+(\d+)?x(\d+)$/;
9+
const allSimdTypes = Object.getOwnPropertyNames(SIMD)
10+
// just to make sure
11+
.filter(simdType => simdRegex.test(simdType))
12+
.map(simdType => {
13+
const result = simdRegex.exec(simdType);
14+
const nLanes = parseInt(result[2]);
15+
const simdInfo = {
16+
makeSimd() {
17+
const args = new Array(nLanes);
18+
for(let i = 0; i < nLanes; ++i) {
19+
args[i] = Math.random() * (1 << 62);
20+
}
21+
return SIMD[simdType](...args);
22+
},
23+
makeStringValue() {
24+
const args = new Array(nLanes);
25+
for(let i = 0; i < nLanes; ++i) {
26+
args[i] = Math.random() * (1 << 62);
27+
}
28+
return `SIMD.${simdType}(${args.join(",")})`;
29+
},
30+
nLanes,
31+
simdType
32+
};
33+
return simdInfo;
34+
});
35+
36+
const allTypes = [0, 1.5, undefined, null, 9223372036854775807, "string", {a: null, b: "b"}];
37+
const tests = allSimdTypes.map(simdInfo => {
38+
return {
39+
name: `memset${simdInfo.simdType}`,
40+
stringValue: simdInfo.makeStringValue(),
41+
v2: simdInfo.makeSimd()
42+
};
43+
});
44+
45+
const types = "Array".split(" ");
46+
47+
let passed = RunMemsetTest(tests, types, allTypes);
48+
49+
print(passed ? "PASSED" : "FAILED");

0 commit comments

Comments
 (0)