Skip to content

Commit ea7f98e

Browse files
rajatdrajatd
committed
[MERGE chakra-core#739] Fixing bug with lossy type spec from uint32 to int32
Merge pull request chakra-core#739 from rajatd:uint32Bug Track lossyness separately for src1 and src2 of an instruction so that we dont miss aggressive int type spec bailouts on one source because the other source was being lossy type spec'ed.
2 parents 066df8a + 7659808 commit ea7f98e

4 files changed

Lines changed: 45 additions & 14 deletions

File tree

‎lib/Backend/GlobOpt.cpp‎

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10462,7 +10462,8 @@ GlobOpt::TypeSpecializeBinary(IR::Instr **pInstr, Value **pSrc1Val, Value **pSrc
1046210462

1046310463
// Type specialize binary operators to int32
1046410464

10465-
bool lossy = true;
10465+
bool src1Lossy = true;
10466+
bool src2Lossy = true;
1046610467
IR::BailOutKind bailOutKind = IR::BailOutInvalid;
1046710468
bool ignoredIntOverflow = this->ignoredIntOverflowForCurrentInstr;
1046810469
bool ignoredNegativeZero = false;
@@ -10746,7 +10747,8 @@ GlobOpt::TypeSpecializeBinary(IR::Instr **pInstr, Value **pSrc1Val, Value **pSrc
1074610747
bailOutKind = IR::BailOutOnDivOfMinInt;
1074710748
}
1074810749

10749-
lossy = false; // Detect -0 on the sources
10750+
src1Lossy = false; // Detect -0 on the sources
10751+
src2Lossy = false;
1075010752

1075110753
opcode = Js::OpCode::Div_I4;
1075210754
bailOutKind |= IR::BailOnDivResultNotInt;
@@ -10969,10 +10971,13 @@ GlobOpt::TypeSpecializeBinary(IR::Instr **pInstr, Value **pSrc1Val, Value **pSrc
1096910971
// If one of the values is a float constant with a value that fits in a uint32 but not an int32,
1097010972
// and the instruction can ignore int overflow, the source value for the purposes of int specialization
1097110973
// would have been changed to an int constant value by ignoring overflow. But, the conversion is still lossy.
10972-
if (!(src1OriginalVal && src1OriginalVal->GetValueInfo()->IsFloatConstant() && src1Val && src1Val->GetValueInfo()->HasIntConstantValue()) &&
10973-
!(src2OriginalVal && src2OriginalVal->GetValueInfo()->IsFloatConstant() && src2Val && src2Val->GetValueInfo()->HasIntConstantValue()))
10974+
if (!(src1OriginalVal && src1OriginalVal->GetValueInfo()->IsFloatConstant() && src1Val && src1Val->GetValueInfo()->HasIntConstantValue()))
1097410975
{
10975-
lossy = false;
10976+
src1Lossy = false;
10977+
}
10978+
if (!(src2OriginalVal && src2OriginalVal->GetValueInfo()->IsFloatConstant() && src2Val && src2Val->GetValueInfo()->HasIntConstantValue()))
10979+
{
10980+
src2Lossy = false;
1097610981
}
1097710982

1097810983
switch(instr->m_opcode)
@@ -11740,26 +11745,29 @@ GlobOpt::TypeSpecializeBinary(IR::Instr **pInstr, Value **pSrc1Val, Value **pSrc
1174011745
}
1174111746

1174211747
Value *src1ValueToSpecialize = src1Val, *src2ValueToSpecialize = src2Val;
11743-
if(lossy)
11748+
// Lossy conversions to int32 must be done based on the original source values. For instance, if one of the values is a
11749+
// float constant with a value that fits in a uint32 but not an int32, and the instruction can ignore int overflow, the
11750+
// source value for the purposes of int specialization would have been changed to an int constant value by ignoring
11751+
// overflow. If we were to specialize the sym using the int constant value, it would be treated as a lossless
11752+
// conversion, but since there may be subsequent uses of the same float constant value that may not ignore overflow,
11753+
// this must be treated as a lossy conversion by specializing the sym using the original float constant value.
11754+
if(src1Lossy)
1174411755
{
11745-
// Lossy conversions to int32 must be done based on the original source values. For instance, if one of the values is a
11746-
// float constant with a value that fits in a uint32 but not an int32, and the instruction can ignore int overflow, the
11747-
// source value for the purposes of int specialization would have been changed to an int constant value by ignoring
11748-
// overflow. If we were to specialize the sym using the int constant value, it would be treated as a lossless
11749-
// conversion, but since there may be subsequent uses of the same float constant value that may not ignore overflow,
11750-
// this must be treated as a lossy conversion by specializing the sym using the original float constant value.
1175111756
src1ValueToSpecialize = src1OriginalVal;
11757+
}
11758+
if (src2Lossy)
11759+
{
1175211760
src2ValueToSpecialize = src2OriginalVal;
1175311761
}
1175411762

1175511763
// Make sure the srcs are specialized
1175611764
src1 = instr->GetSrc1();
11757-
this->ToInt32(instr, src1, this->currentBlock, src1ValueToSpecialize, nullptr, lossy);
11765+
this->ToInt32(instr, src1, this->currentBlock, src1ValueToSpecialize, nullptr, src1Lossy);
1175811766

1175911767
if (!skipSrc2)
1176011768
{
1176111769
src2 = instr->GetSrc2();
11762-
this->ToInt32(instr, src2, this->currentBlock, src2ValueToSpecialize, nullptr, lossy);
11770+
this->ToInt32(instr, src2, this->currentBlock, src2ValueToSpecialize, nullptr, src2Lossy);
1176311771
}
1176411772

1176511773
if(bailOutKind != IR::BailOutInvalid && !this->IsLoopPrePass())

‎test/Optimizer/bug7100343.baseline‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
1
2+
1
3+
1

‎test/Optimizer/bug7100343.js‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
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+
function foo(i1)
7+
{
8+
var d2 = 16777217;
9+
return (!i1 >>> ((d2 % ~~295147905179352830000) + 4268133759)) | 0;
10+
}
11+
print(foo());
12+
print(foo());
13+
print(foo());

‎test/Optimizer/rlexe.xml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1346,4 +1346,11 @@
13461346
<compile-flags>-lic:1 -off:nativearray</compile-flags>
13471347
</default>
13481348
</test>
1349+
<test>
1350+
<default>
1351+
<files>bug7100343.js</files>
1352+
<compile-flags>-mic:1 -off:simplejit -force:rejit</compile-flags>
1353+
<baseline>bug7100343.baseline</baseline>
1354+
</default>
1355+
</test>
13491356
</regress-exe>

0 commit comments

Comments
 (0)