Skip to content

Commit 9ab223c

Browse files
ConstellationConstellation
committed
[JSC] B3 reduce-double-to-float should reduce only when constant double is canonical one to reduced float value
https://bugs.webkit.org/show_bug.cgi?id=224403 <rdar://problem/76259599> Reviewed by Mark Lam. When reducing double-constant value to float in B3, we should check whether the double value is a canonical one which can be converted back from the reduced float value. For example, double 1.1 is not the one since it is truncated into float 1.1 by removing some bits. static_cast<double>(static_cast<float>(1.1)) != 1.1 Reducing such a double to float changes the semantics. * b3/B3ConstDoubleValue.cpp: (JSC::B3::ConstDoubleValue::dumpMeta const): * b3/B3ConstFloatValue.cpp: (JSC::B3::ConstFloatValue::dumpMeta const): * b3/B3ReduceDoubleToFloat.cpp: * b3/B3ReduceStrength.cpp: * b3/testb3.h: (populateWithInterestingValues): * b3/testb3_1.cpp: (run): * b3/testb3_3.cpp: (testConvertDoubleToFloatToDouble): (testConvertDoubleToFloatEqual): * ftl/FTLLowerDFGToB3.cpp: (JSC::FTL::DFG::JSC_DEFINE_JIT_OPERATION_WITH_ATTRIBUTES): (JSC::FTL::DFG::LowerDFGToB3::crash): (JSC::FTL::DFG::ftlUnreachable): Deleted. Canonical link: https://commits.webkit.org/236371@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@275800 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent b3d023b commit 9ab223c

9 files changed

Lines changed: 89 additions & 7 deletions

File tree

‎Source/JavaScriptCore/ChangeLog‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,37 @@
1+
2021-04-10 Yusuke Suzuki <ysuzuki@apple.com>
2+
3+
[JSC] B3 reduce-double-to-float should reduce only when constant double is canonical one to reduced float value
4+
https://bugs.webkit.org/show_bug.cgi?id=224403
5+
<rdar://problem/76259599>
6+
7+
Reviewed by Mark Lam.
8+
9+
When reducing double-constant value to float in B3, we should check whether the double value is a canonical one
10+
which can be converted back from the reduced float value. For example, double 1.1 is not the one since it is truncated
11+
into float 1.1 by removing some bits.
12+
13+
static_cast<double>(static_cast<float>(1.1)) != 1.1
14+
15+
Reducing such a double to float changes the semantics.
16+
17+
* b3/B3ConstDoubleValue.cpp:
18+
(JSC::B3::ConstDoubleValue::dumpMeta const):
19+
* b3/B3ConstFloatValue.cpp:
20+
(JSC::B3::ConstFloatValue::dumpMeta const):
21+
* b3/B3ReduceDoubleToFloat.cpp:
22+
* b3/B3ReduceStrength.cpp:
23+
* b3/testb3.h:
24+
(populateWithInterestingValues):
25+
* b3/testb3_1.cpp:
26+
(run):
27+
* b3/testb3_3.cpp:
28+
(testConvertDoubleToFloatToDouble):
29+
(testConvertDoubleToFloatEqual):
30+
* ftl/FTLLowerDFGToB3.cpp:
31+
(JSC::FTL::DFG::JSC_DEFINE_JIT_OPERATION_WITH_ATTRIBUTES):
32+
(JSC::FTL::DFG::LowerDFGToB3::crash):
33+
(JSC::FTL::DFG::ftlUnreachable): Deleted.
34+
135
2021-04-10 Mark Lam <mark.lam@apple.com>
236

337
Enable VMTraps checks in RETURN_IF_EXCEPTION.

‎Source/JavaScriptCore/b3/B3ConstDoubleValue.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ TriState ConstDoubleValue::equalOrUnorderedConstant(const Value* other) const
194194
void ConstDoubleValue::dumpMeta(CommaPrinter& comma, PrintStream& out) const
195195
{
196196
out.print(comma);
197-
out.printf("%le", m_value);
197+
out.printf("%le(%llu)", m_value, bitwise_cast<uint64_t>(m_value));
198198
}
199199

200200
} } // namespace JSC::B3

‎Source/JavaScriptCore/b3/B3ConstFloatValue.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ TriState ConstFloatValue::equalOrUnorderedConstant(const Value* other) const
186186
void ConstFloatValue::dumpMeta(CommaPrinter& comma, PrintStream& out) const
187187
{
188188
out.print(comma);
189-
out.printf("%le", m_value);
189+
out.printf("%le(%u)", m_value, bitwise_cast<uint32_t>(m_value));
190190
}
191191

192192
} } // namespace JSC::B3

‎Source/JavaScriptCore/b3/B3ReduceDoubleToFloat.cpp‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,8 +213,20 @@ class DoubleToFloatReduction {
213213
if (value->opcode() == FloatToDouble)
214214
return true;
215215

216-
if (value->hasDouble())
217-
return true; // Double constant truncated to float.
216+
if (value->hasDouble()) {
217+
// When comparing double and float, some range of double values will be truncated into one float.
218+
// So we need to ensure that this double value is one-on-one representation to the original double.
219+
// Let's consider the case,
220+
//
221+
// Equal(Double: 1.1, FloatToDouble(Float: 1.1))
222+
//
223+
// This should be false. This is because
224+
//
225+
// static_cast<double>(static_cast<float>(1.1)) != 1.1
226+
//
227+
double constValue = value->asDouble();
228+
return isIdentical(static_cast<double>(static_cast<float>(constValue)), constValue);
229+
}
218230

219231
if (value->opcode() == Phi) {
220232
return value->type() == Float

‎Source/JavaScriptCore/b3/B3ReduceStrength.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1687,6 +1687,10 @@ class ReduceStrength {
16871687
break;
16881688

16891689
case FloatToDouble:
1690+
// We cannot convert some FloatToDouble(DoubleToFloat(value)) to value, because double-to-float will trancate some range of double values into one float.
1691+
// Example:
1692+
// static_cast<double>(static_cast<float>(1.1)) != 1.1
1693+
16901694
// Turn this: FloatToDouble(constant)
16911695
// Into this: ConstDouble(constant)
16921696
if (Value* constant = m_value->child(0)->floatToDoubleConstant(m_proc)) {

‎Source/JavaScriptCore/b3/testb3.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,8 @@ void populateWithInterestingValues(Vector<B3Operand<FloatType>>& operands)
290290
operands.append({ "-0.6", static_cast<FloatType>(-0.6) });
291291
operands.append({ "1.", static_cast<FloatType>(1.) });
292292
operands.append({ "-1.", static_cast<FloatType>(-1.) });
293+
operands.append({ "1.1", static_cast<FloatType>(1.1) });
294+
operands.append({ "-1.1", static_cast<FloatType>(-1.1) });
293295
operands.append({ "2.", static_cast<FloatType>(2.) });
294296
operands.append({ "-2.", static_cast<FloatType>(-2.) });
295297
operands.append({ "M_PI", static_cast<FloatType>(M_PI) });
@@ -614,7 +616,9 @@ void testConvertDoubleToFloatMem(double value);
614616
void testConvertFloatToDoubleArg(float value);
615617
void testConvertFloatToDoubleImm(float value);
616618
void testConvertFloatToDoubleMem(float value);
619+
void testConvertDoubleToFloatToDouble(double value);
617620
void testConvertDoubleToFloatToDoubleToFloat(double value);
621+
void testConvertDoubleToFloatEqual(double value);
618622
void testLoadFloatConvertDoubleConvertFloatStoreFloat(float value);
619623
void testFroundArg(double value);
620624
void testFroundMem(double value);

‎Source/JavaScriptCore/b3/testb3_1.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,9 @@ void run(const char* filter)
309309
RUN_UNARY(testConvertFloatToDoubleArg, floatingPointOperands<float>());
310310
RUN_UNARY(testConvertFloatToDoubleImm, floatingPointOperands<float>());
311311
RUN_UNARY(testConvertFloatToDoubleMem, floatingPointOperands<float>());
312+
RUN_UNARY(testConvertDoubleToFloatToDouble, floatingPointOperands<double>());
312313
RUN_UNARY(testConvertDoubleToFloatToDoubleToFloat, floatingPointOperands<double>());
314+
RUN_UNARY(testConvertDoubleToFloatEqual, floatingPointOperands<double>());
313315
RUN_UNARY(testStoreFloat, floatingPointOperands<double>());
314316
RUN_UNARY(testStoreDoubleConstantAsFloat, floatingPointOperands<double>());
315317
RUN_UNARY(testLoadFloatConvertDoubleConvertFloatStoreFloat, floatingPointOperands<float>());

‎Source/JavaScriptCore/b3/testb3_3.cpp‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2438,6 +2438,18 @@ void testConvertFloatToDoubleMem(float value)
24382438
CHECK(isIdentical(compileAndRun<double>(proc, &value), static_cast<double>(value)));
24392439
}
24402440

2441+
void testConvertDoubleToFloatToDouble(double value)
2442+
{
2443+
Procedure proc;
2444+
BasicBlock* root = proc.addBlock();
2445+
Value* argument = root->appendNew<ArgumentRegValue>(proc, Origin(), FPRInfo::argumentFPR0);
2446+
Value* asFloat = root->appendNew<Value>(proc, DoubleToFloat, Origin(), argument);
2447+
Value* asDouble = root->appendNew<Value>(proc, FloatToDouble, Origin(), asFloat);
2448+
root->appendNewControlValue(proc, Return, Origin(), asDouble);
2449+
2450+
CHECK(isIdentical(compileAndRun<double>(proc, value), static_cast<double>(static_cast<float>(value))));
2451+
}
2452+
24412453
void testConvertDoubleToFloatToDoubleToFloat(double value)
24422454
{
24432455
Procedure proc;
@@ -2451,6 +2463,20 @@ void testConvertDoubleToFloatToDoubleToFloat(double value)
24512463
CHECK(isIdentical(compileAndRun<float>(proc, value), static_cast<float>(value)));
24522464
}
24532465

2466+
void testConvertDoubleToFloatEqual(double value)
2467+
{
2468+
Procedure proc;
2469+
BasicBlock* root = proc.addBlock();
2470+
Value* argument = root->appendNew<ArgumentRegValue>(proc, Origin(), FPRInfo::argumentFPR0);
2471+
Value* asFloat = root->appendNew<Value>(proc, DoubleToFloat, Origin(), argument);
2472+
Value* asDouble = root->appendNew<Value>(proc, FloatToDouble, Origin(), asFloat);
2473+
Value* constant = root->appendNew<ConstDoubleValue>(proc, Origin(), value);
2474+
Value* argsAreEqual = root->appendNew<Value>(proc, Equal, Origin(), asDouble, constant);
2475+
root->appendNewControlValue(proc, Return, Origin(), argsAreEqual);
2476+
2477+
CHECK(compileAndRun<bool>(proc, value) == (static_cast<double>(static_cast<float>(value)) == value));
2478+
}
2479+
24542480
void testLoadFloatConvertDoubleConvertFloatStoreFloat(float value)
24552481
{
24562482
Procedure proc;

‎Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,8 @@ namespace {
119119
std::atomic<int> compileCounter;
120120

121121
#if ASSERT_ENABLED
122-
NO_RETURN_DUE_TO_CRASH static void ftlUnreachable(
123-
CodeBlock* codeBlock, BlockIndex blockIndex, unsigned nodeIndex)
122+
static NO_RETURN_DUE_TO_CRASH JSC_DECLARE_JIT_OPERATION(ftlUnreachable, void, (CodeBlock* codeBlock, BlockIndex blockIndex, unsigned nodeIndex));
123+
JSC_DEFINE_JIT_OPERATION_WITH_ATTRIBUTES(ftlUnreachable, NO_RETURN_DUE_TO_CRASH, void, (CodeBlock* codeBlock, BlockIndex blockIndex, unsigned nodeIndex))
124124
{
125125
dataLog("Crashing in thought-to-be-unreachable FTL-generated code for ", pointerDump(codeBlock), " at basic block #", blockIndex);
126126
if (nodeIndex != UINT_MAX)
@@ -19907,7 +19907,7 @@ class LowerDFGToB3 {
1990719907
#else // ASSERT_ENABLED
1990819908
m_out.call(
1990919909
Void,
19910-
m_out.constIntPtr(ftlUnreachable),
19910+
m_out.operation(ftlUnreachable),
1991119911
// We don't want the CodeBlock to have a weak pointer to itself because
1991219912
// that would cause it to always get collected.
1991319913
m_out.constIntPtr(bitwise_cast<intptr_t>(codeBlock())), m_out.constInt32(blockIndex),

0 commit comments

Comments
 (0)