Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DFG should support tuples #11086

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion JSTests/stress/for-in-redefine-enumerable.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ function assert(x) {

function shouldBe(actual, expected) {
if (actual !== expected)
throw new Error(`Bad value: ${actual}.`);
throw new Error(`Bad value: ${actual} expected ${expected}.`);
}

const enumDesc = { value: 0, writable: true, enumerable: true, configurable: true };
Expand All @@ -15,13 +15,16 @@ const dontEnumDesc = { value: 0, writable: true, enumerable: false, configurable
(() => {
function test() {
var arr = Object.defineProperties([0, 0, 0], { 1: dontEnumDesc });
var count = 0;
for (var i in arr) {
count++;
assert(i in arr);
shouldBe(arr[i], 0);
++arr[i];
if (i === "0")
Object.defineProperties(arr, { 1: enumDesc, 2: dontEnumDesc });
}
shouldBe(count, 1);
shouldBe(arr[0], 1);
shouldBe(arr[1], 0);
shouldBe(arr[2], 0);
Expand Down
12 changes: 9 additions & 3 deletions Source/JavaScriptCore/b3/B3LowerToAir.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#if ENABLE(B3_JIT)

#include "AirBlockInsertionSet.h"
#include "AirCCallSpecial.h"
#include "AirCode.h"
#include "AirHelpers.h"
#include "AirInsertionSet.h"
Expand Down Expand Up @@ -144,6 +145,7 @@ class LowerToAir {
}
case Get:
case Patchpoint:
case B3::CCall:
case BottomTuple: {
if (value->type().isTuple())
ensureTupleTmps(value, m_tupleValueToTmps);
Expand Down Expand Up @@ -446,6 +448,7 @@ class LowerToAir {
switch (tupleValue->opcode()) {
case Phi:
case Patchpoint:
case B3::CCall:
case BottomTuple: {
return m_tupleValueToTmps.find(tupleValue)->value;
}
Expand Down Expand Up @@ -4457,7 +4460,7 @@ class LowerToAir {
case B3::CCall: {
CCallValue* cCall = m_value->as<CCallValue>();

Inst inst(m_isRare ? Air::ColdCCall : Air::CCall, cCall);
Inst inst(m_isRare ? Air::ColdCCall : Air::CCall, cCall, Arg::special(m_code.cCallSpecial()));

// We have a ton of flexibility regarding the callee argument, but currently, we don't
// use it yet. It gets weird for reasons:
Expand All @@ -4470,8 +4473,11 @@ class LowerToAir {
// FIXME: https://bugs.webkit.org/show_bug.cgi?id=151052
inst.args.append(tmp(cCall->child(0)));

if (cCall->type() != Void)
inst.args.append(tmp(cCall));
if (cCall->type() != Void) {
forEachImmOrTmp(cCall, [&] (Arg arg, Type, unsigned) {
inst.args.append(arg.tmp());
});
}

for (unsigned i = 1; i < cCall->numChildren(); ++i)
inst.args.append(immOrTmp(cCall->child(i)));
Expand Down
9 changes: 8 additions & 1 deletion Source/JavaScriptCore/b3/B3Type.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,20 @@ inline bool Type::isVector() const
return kind() == V128;
}

inline Type pointerType()
constexpr Type pointerType()
{
if (is32Bit())
return Int32;
return Int64;
}

constexpr Type registerType()
{
if (isRegister64Bit())
return Int64;
return Int32;
}

inline size_t sizeofType(Type type)
{
switch (type.kind()) {
Expand Down
8 changes: 8 additions & 0 deletions Source/JavaScriptCore/b3/B3Validate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -674,6 +674,14 @@ class Validater {
VALIDATE(!value->kind().hasExtraBits(), ("At ", *value));
VALIDATE(value->numChildren() >= 1, ("At ", *value));
VALIDATE(value->child(0)->type() == pointerType(), ("At ", *value));
if (value->type().isTuple()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add comment that currently we are only supporting two-element case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

// FIXME: Right now we only support a pair of register sized values since on every calling
// convention we support that's returned in returnValueGPR/returnValueGPR2, respectively.
VALIDATE(m_procedure.resultCount(value->type()) == 2, ("At ", *value));
VALIDATE(m_procedure.typeAtOffset(value->type(), 0) == registerType(), ("At ", *value));
VALIDATE(m_procedure.typeAtOffset(value->type(), 1) == registerType(), ("At ", *value));
}

break;
case Patchpoint:
VALIDATE(!value->kind().hasExtraBits(), ("At ", *value));
Expand Down
29 changes: 20 additions & 9 deletions Source/JavaScriptCore/b3/air/AirCCallingConvention.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ void marshallCCallArgumentImpl(Vector<Arg>& result, unsigned& argumentCount, uns
// In the rare case when the Arg width does not match the argument width
// (32-bit arm passing a 64-bit argument), we respect the width needed
// for each stack access:
slotSize = bytesForWidth(cCallArgumentRegisterWidth(child));
slotSize = bytesForWidth(cCallArgumentRegisterWidth(child->type()));

// but the logical stack slot uses the natural alignment of the argument
slotAlignment = sizeofType(child->type());
Expand Down Expand Up @@ -103,7 +103,7 @@ Vector<Arg> computeCCallingConvention(Code& code, CCallValue* value)
return result;
}

size_t cCallResultCount(CCallValue* value)
size_t cCallResultCount(Code& code, CCallValue* value)
{
switch (value->type().kind()) {
case Void:
Expand All @@ -113,7 +113,12 @@ size_t cCallResultCount(CCallValue* value)
return 2;
return 1;
case Tuple:
RELEASE_ASSERT_NOT_REACHED();
// We only support tuples that return exactly two register sized ints.
UNUSED_PARAM(code);
ASSERT(code.proc().resultCount(value->type()) == 2);
ASSERT(code.proc().typeAtOffset(value->type(), 0) == pointerType());
ASSERT(code.proc().typeAtOffset(value->type(), 1) == pointerType());
return 2;
default:
return 1;

Expand All @@ -136,19 +141,19 @@ size_t cCallArgumentRegisterCount(const Value* value)
}
}

Width cCallArgumentRegisterWidth(const Value* value)
Width cCallArgumentRegisterWidth(Type type)
{
if constexpr (is32Bit()) {
if (value->type() == Int64)
if (type == Int64)
return Width32;
}

return widthForType(value->type());
return widthForType(type);
}

Tmp cCallResult(CCallValue* value, unsigned index)
Tmp cCallResult(Code& code, CCallValue* value, unsigned index)
{
ASSERT_UNUSED(index, index <= (is64Bit() ? 1 : 2));
ASSERT(index < 2);
switch (value->type().kind()) {
case Void:
return Tmp();
Expand All @@ -160,9 +165,15 @@ Tmp cCallResult(CCallValue* value, unsigned index)
return Tmp(GPRInfo::returnValueGPR);
case Float:
case Double:
ASSERT(!index);
return Tmp(FPRInfo::returnValueFPR);
case V128:
case Tuple:
ASSERT_UNUSED(code, code.proc().resultCount(value->type()) == 2);
// We only support functions that return each parameter in its own register for now.
ASSERT(code.proc().typeAtOffset(value->type(), 0) == registerType());
ASSERT(code.proc().typeAtOffset(value->type(), 1) == registerType());
return index ? Tmp(GPRInfo::returnValueGPR2) : Tmp(GPRInfo::returnValueGPR);
case V128:
break;
}

Expand Down
6 changes: 3 additions & 3 deletions Source/JavaScriptCore/b3/air/AirCCallingConvention.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class Code;

Vector<Arg> computeCCallingConvention(Code&, CCallValue*);

size_t cCallResultCount(CCallValue*);
size_t cCallResultCount(Code&, CCallValue*);

/*
* On some platforms (well, on 32-bit platforms,) C functions can take arguments
Expand All @@ -54,9 +54,9 @@ size_t cCallResultCount(CCallValue*);
// Return the number of Air::Args needed to marshall this Value to the C function
size_t cCallArgumentRegisterCount(const Value*);
// Return the width of the individual Air::Args needed to marshall this value
Width cCallArgumentRegisterWidth(const Value*);
Width cCallArgumentRegisterWidth(Type);

Tmp cCallResult(CCallValue*, unsigned);
Tmp cCallResult(Code&, CCallValue*, unsigned);

Inst buildCCall(Code&, Value* origin, const Vector<Arg>&);

Expand Down
27 changes: 20 additions & 7 deletions Source/JavaScriptCore/b3/air/AirCustom.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "AirCCallingConvention.h"
#include "AirInstInlines.h"
#include "B3CCallValue.h"
#include "B3ProcedureInlines.h"
#include "B3ValueInlines.h"
#include "CCallHelpers.h"

Expand Down Expand Up @@ -65,8 +66,14 @@ bool CCallCustom::isValidForm(Inst& inst)
if (!value)
return false;

size_t resultCount = cCallResultCount(value);
size_t expectedArgCount = resultCount;
if (!inst.args[0].isSpecial())
return false;

Special* special = inst.args[0].special();
Code& code = special->code();

size_t resultCount = cCallResultCount(code, value);
size_t expectedArgCount = resultCount + 1; // first Arg is always CCallSpecial.
for (Value* child : value->children()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, it is including callee itself.

ASSERT(child->type() != Tuple);
expectedArgCount += cCallArgumentRegisterCount(child);
Expand All @@ -76,22 +83,28 @@ bool CCallCustom::isValidForm(Inst& inst)
return false;

// The arguments can only refer to the stack, tmps, or immediates.
for (Arg& arg : inst.args) {
for (unsigned i = inst.args.size() - 1; i; --i) {
Arg arg = inst.args[i];
if (!arg.isTmp() && !arg.isStackMemory() && !arg.isSomeImm())
return false;
}

// Callee
if (!inst.args[0].isGP())
if (!inst.args[1].isGP())
return false;

unsigned offset = 1;
unsigned offset = 2;

// If there is a result then it cannot be an immediate.
for (size_t i = 0 ; i < resultCount; ++i) {
for (size_t i = 0; i < resultCount; ++i) {
if (inst.args[offset].isSomeImm())
return false;
if (!inst.args[offset].canRepresent(value))

if (value->type().isTuple()) {
Type type = code.proc().typeAtOffset(value->type(), i);
if (!inst.args[offset].canRepresent(type))
return false;
} else if (!inst.args[offset].canRepresent(value))
return false;
offset++;
}
Expand Down
15 changes: 10 additions & 5 deletions Source/JavaScriptCore/b3/air/AirCustom.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,19 +134,24 @@ struct CCallCustom : public CommonCustomBase<CCallCustom> {
{
CCallValue* value = inst.origin->as<CCallValue>();

unsigned index = 0;
Code& code = inst.args[0].special()->code();

// Skip the CCallSpecial Arg.
unsigned index = 1;

auto next = [&](Arg::Role role, Bank bank, Width width) {
functor(inst.args[index++], role, bank, width);
};

next(Arg::Use, GP, pointerWidth()); // callee

for (size_t n = cCallResultCount(value); n; --n) {
size_t resultCount = cCallResultCount(code, value);
for (size_t n = 0; n < resultCount; ++n) {
Type type = value->type().isTuple() ? code.proc().typeAtOffset(value->type(), n) : value->type();
next(
Arg::Def,
bankForType(value->type()),
cCallArgumentRegisterWidth(value)
bankForType(type),
cCallArgumentRegisterWidth(type)
);
}

Expand All @@ -156,7 +161,7 @@ struct CCallCustom : public CommonCustomBase<CCallCustom> {
next(
Arg::Use,
bankForType(child->type()),
cCallArgumentRegisterWidth(child)
cCallArgumentRegisterWidth(child->type())
);
}
}
Expand Down
32 changes: 20 additions & 12 deletions Source/JavaScriptCore/b3/air/AirLowerAfterRegAlloc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include "AirRegLiveness.h"
#include "AirPhaseScope.h"
#include "B3CCallValue.h"
#include "B3ProcedureInlines.h"
#include "B3ValueInlines.h"
#include "RegisterSet.h"
#include <wtf/HashMap.h>
Expand Down Expand Up @@ -193,13 +194,17 @@ void lowerAfterRegAlloc(Code& code)
ScalarRegisterSet preUsed = liveRegs.buildScalarRegisterSet();
ScalarRegisterSet postUsed = preUsed;
Vector<Arg> destinations = computeCCallingConvention(code, value);
Tmp result = cCallResult(value, 0);
Arg originalResult = result ? inst.args[1] : Arg();
Vector<Tmp, 2> results;
Vector<Arg, 2> originalResults;
for (unsigned i = 0; i < cCallResultCount(code, value); ++i) {
results.append(cCallResult(code, value, i));
originalResults.append(inst.args[i + 2]);
}

Vector<ShufflePair> pairs;
for (unsigned i = 0; i < destinations.size(); ++i) {
Value* child = value->child(i);
Arg src = inst.args[result ? (i >= 1 ? i + 1 : i) : i ];
Arg src = inst.args[i >= 1 ? i + results.size() + 1 : i + 1];
Arg dst = destinations[i];
Width width = widthForType(child->type());
pairs.append(ShufflePair(src, dst, width));
Expand All @@ -217,8 +222,10 @@ void lowerAfterRegAlloc(Code& code)

// Also need to save all live registers. Don't need to worry about the result
// register.
if (originalResult.isReg())
regsToSave.remove(originalResult.reg());
for (Arg originalResult : originalResults) {
if (originalResult.isReg())
regsToSave.remove(originalResult.reg());
}
Vector<StackSlot*> stackSlots;
regsToSave.forEachWithWidth(
[&] (Reg reg, Width width) {
Expand Down Expand Up @@ -251,15 +258,16 @@ void lowerAfterRegAlloc(Code& code)
ASSERT(stackSlot->byteSize() >= bytesForWidth(width));
pairs.append(ShufflePair(Arg::stack(stackSlot), arg, width));
});
if (result) {
ShufflePair pair(result, originalResult, widthForType(value->type()));
for (unsigned i = 0; i < results.size(); ++i) {
Type type = value->type().isTuple() ? code.proc().typeAtOffset(value->type(), i) : value->type();
ShufflePair pair(results[i], originalResults[i], widthForType(type));
pairs.append(pair);
}

// For finding scratch registers, we need to account for the possibility that
// the result is dead.
if (originalResult.isReg())
postUsed.add(originalResult.reg(), IgnoreVectors);
// For finding scratch registers, we need to account for the possibility that
// the result is dead.
if (originalResults[i].isReg())
postUsed.add(originalResults[i].reg(), IgnoreVectors);
}

gpScratch = getScratches(postUsed, GP);
fpScratch = getScratches(postUsed, FP);
Expand Down