Bug 194036 – [WebAssembly] Write a new register allocator for Air O0 and make BBQ use it

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Saam Barati]

...

[Saam Barati]

Created attachment 360608 [details]
WIP

[Saam Barati]

Created attachment 360647 [details]
patch

[EWS Watchlist]

Attachment 360647 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp:1776:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp:2061:  One line control clauses should not use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp:2197:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp:2219:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp:3029:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
ERROR: Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp:3383:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
Total errors found: 6 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Robin Morisset]

The changelog does not match this bug.

[Saam Barati]

Created attachment 360667 [details]
WIP

[Saam Barati]

Created attachment 360907 [details]
WIP

Code is quite the mess at the moment, but this is another 10% speedup on the wasm subset of tests in JetStream 2.

[Saam Barati]

Created attachment 360947 [details]
WIP

[Saam Barati]

Created attachment 360948 [details]
WIP

[Saam Barati]

Created attachment 361270 [details]
WIP

almost there

[Saam Barati]

Created attachment 361481 [details]
patch

[EWS Watchlist]

Attachment 361481 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.h:55:  The parameter name "tmp" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.h:55:  The parameter name "reg" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.h:56:  The parameter name "tmp" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.h:56:  The parameter name "reg" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.h:57:  The parameter name "tmp" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.h:57:  The parameter name "reg" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.h:59:  The parameter name "tmp" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.h:59:  The parameter name "bank" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 8 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Saam Barati]

Created attachment 361488 [details]
patch

[EWS Watchlist]

Comment on attachment 361488 [details]
patch

Attachment 361488 [details] did not pass jsc-ews (mac):
Output: https://webkit-queues.webkit.org/results/11075654

New failing tests:
microbenchmarks/register-pressure-from-osr.js.ftl-no-cjit-b3o0
apiTests

[Saam Barati]

Created attachment 361496 [details]
patch

[Saam Barati]

Created attachment 361497 [details]
patch

[Tadeu Zagallo]

Comment on attachment 361497 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=361497&action=review

I don't really know AIR, but I had a couple nits.

> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:47
> +void GenerateAndAllocateRegisters::buildLiveRanges(UnifiedTmpLiveness& liveness)

I don’t think you need to use std::max here. The only values that are stored in the map come from m_globalInstIndex, which is monotonically increasing, so I don’t think it’s possible for m_liveRangeEnd to ever contain a value bigger than m_globalInstIndex.

> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:131
> +    if (tmp.bank() == GP)

nit: why not `tmp.isGP()`?

> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:222
> +    for (unsigned i = 0; i < m_registers[bank].size(); ++i) {

nit: for (Reg reg : m_registers[bank])

> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:521
> +                // available registers, we convert anything that accepts stack to be a stack addr

Wouldn't this potentially spill more than necessary? e.g. if you have 2 args that accept stack but only 1 register left, wouldn't this spill both args?

> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:590
> +                bool everySuccessorGetsOurRegisterState = true;

nice!

[Saam Barati]

Comment on attachment 361497 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=361497&action=review

>> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:47
>> +void GenerateAndAllocateRegisters::buildLiveRanges(UnifiedTmpLiveness& liveness)
> 
> I don’t think you need to use std::max here. The only values that are stored in the map come from m_globalInstIndex, which is monotonically increasing, so I don’t think it’s possible for m_liveRangeEnd to ever contain a value bigger than m_globalInstIndex.

yeah good point.

>> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:131
>> +    if (tmp.bank() == GP)
> 
> nit: why not `tmp.isGP()`?

sounds reasonable.

>> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:222
>> +    for (unsigned i = 0; i < m_registers[bank].size(); ++i) {
> 
> nit: for (Reg reg : m_registers[bank])

sounds good.

>> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:521
>> +                // available registers, we convert anything that accepts stack to be a stack addr
> 
> Wouldn't this potentially spill more than necessary? e.g. if you have 2 args that accept stack but only 1 register left, wouldn't this spill both args?

No. This only comes into play when an Inst itself has more args that the size of the register file. E.g, OSR exit stackmaps may have arbitrarily large number of args. Or Tail calls may also have arbitrarily large number of args.

[Saam Barati]

Created attachment 361716 [details]
patch

[EWS Watchlist]

Comment on attachment 361716 [details]
patch

Attachment 361716 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/11114791

New failing tests:
http/tests/inspector/network/resource-initiatorNode.html

[EWS Watchlist]

Created attachment 361742 [details]
Archive of layout-test-results from ews112 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[Yusuke Suzuki]

Comment on attachment 361716 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=361716&action=review

r=me. I'm not an expert of Air thing now, but I'm attempting to understand what is going on. Exciting!

> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:72
> +void GenerateAndAllocateRegisters::gatherTerminalPatchSpills()

My understanding is this is inserting basic block which will contain spill code. If so, I think "insertBlocksForFlush" or something like that seems better. What do you think of?

> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:102
> +            m_blocksForPatchSpilling.add(newBlock, PatchSpillData { CCallHelpers::Jump(), CCallHelpers::Label(), needToDef });

Use `WTFMove(needToDef)`

> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:327
> +    IndexMap<BasicBlock*, IndexMap<Reg, Tmp>> currentAllocationMap(m_code.size());
> +    {
> +        IndexMap<Reg, Tmp> defaultCurrentAllocation(Reg::maxIndex() + 1);
> +        for (BasicBlock* block : m_code) {
> +            if (block == m_code[0]) // Handled below.
> +                continue;
> +            currentAllocationMap[block] = defaultCurrentAllocation;
> +        }
> +
> +        for (Tmp tmp : liveness.liveAtHead(m_code[0])) {
> +            if (!tmp.isReg())
> +                continue;
> +            defaultCurrentAllocation[tmp.reg()] = tmp;
> +        }
> +        currentAllocationMap[m_code[0]] = defaultCurrentAllocation;
> +    }

OK, clearing current allocation status first.

> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:381
> +        {
> +            auto iter = m_blocksForPatchSpilling.find(block);
> +            if (iter != m_blocksForPatchSpilling.end()) {
> +                auto& data = iter->value;
> +                data.jump = m_jit->jump();
> +                data.continueLabel = m_jit->label();
> +            }
> +        }

OK, if the given block is newly inserted one for spilling, jump to the code for spills, and then, back to the continueLabel.

> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:432
> +                if (tmp.isReg()) {

My understanding of Air::Tmp::isReg is that it has specific register requirement (like, registers for x86 div), is it right?

> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:466
> +                if (inst.kind.opcode == Patch || (nextInst && nextInst->kind.opcode == Patch)) {
> +                    if (inst.kind.opcode == Patch)
> +                        clobberedRegisters.merge(inst.extraClobberedRegs());
> +                    if (nextInst && nextInst->kind.opcode == Patch)
> +                        clobberedRegisters.merge(nextInst->extraEarlyClobberedRegs());
> +
> +                    clobberedRegisters.filter(m_allowedRegisters);
> +                    clobberedRegisters.exclude(m_namedDefdRegs);
> +                    m_namedDefdRegs.merge(clobberedRegisters);
> +                }

My understanding is that Patch operation can declare early clobber thing, which needs to be considered here since it is early clobbered, correct?

> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:620
> +                if (inst.kind.opcode == Jump && block->successorBlock(0) == m_code.findNextBlock(block))
> +                    needsToGenerate = false;

OK, fallthrough code.

> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:636
> +                    CCallHelpers::Jump jump = block->last().generate(*m_jit, context);

`inst.generate(...)` seems better for consistency here.

> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:689
> +    for (auto& entry : m_blocksForPatchSpilling) {
> +        entry.value.jump.linkTo(m_jit->label(), m_jit);
> +        const HashMap<Tmp, Arg*>& spills = entry.value.defdTmps;
> +        for (auto& entry : spills) {
> +            Arg* arg = entry.value;
> +            if (!arg->isTmp())
> +                continue;
> +            Tmp originalTmp = entry.key;
> +            Tmp currentTmp = arg->tmp();
> +            ASSERT_WITH_MESSAGE(currentTmp.isReg(), "We already did register allocation so we should have assigned this Tmp to a register.");
> +            flush(originalTmp, currentTmp.reg());
> +        }
> +        m_jit->jump().linkTo(entry.value.continueLabel, m_jit);
> +    }

OK, emitting spill code.

> Source/JavaScriptCore/wasm/WasmFunctionParser.h:172
> +    m_context.didKill(right);

This is really nice. But I think we can do this semi-automatic by,

1. Recording expressions in Vector<ExpressionType, X> or something in WASM_TRY_POP_EXPRESSION_STACK_INTO,
2. Kill the recorded expressions when returning from parseExpression.

What do you think of?

[Saam Barati]

Comment on attachment 361716 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=361716&action=review

>> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:102
>> +            m_blocksForPatchSpilling.add(newBlock, PatchSpillData { CCallHelpers::Jump(), CCallHelpers::Label(), needToDef });
> 
> Use `WTFMove(needToDef)`

We can’t. This is copied in a loop

>> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:432
>> +                if (tmp.isReg()) {
> 
> My understanding of Air::Tmp::isReg is that it has specific register requirement (like, registers for x86 div), is it right?

Yup exactly. When generating Air code, you can specify Tmps to be locked to a specific register. So logically, before register allocation, a Tmp is either a named FP/GP register, or an unnamed numbered Tmp

>> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:466
>> +                }
> 
> My understanding is that Patch operation can declare early clobber thing, which needs to be considered here since it is early clobbered, correct?

Yeah

>> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:636
>> +                    CCallHelpers::Jump jump = block->last().generate(*m_jit, context);
> 
> `inst.generate(...)` seems better for consistency here.

Sounds good

[Saam Barati]

Comment on attachment 361716 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=361716&action=review

>> Source/JavaScriptCore/wasm/WasmFunctionParser.h:172
>> +    m_context.didKill(right);
> 
> This is really nice. But I think we can do this semi-automatic by,
> 
> 1. Recording expressions in Vector<ExpressionType, X> or something in WASM_TRY_POP_EXPRESSION_STACK_INTO,
> 2. Kill the recorded expressions when returning from parseExpression.
> 
> What do you think of?

I think you’re right for most places. Let me read the code again

[Robin Morisset]

Comment on attachment 361716 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=361716&action=review

Just a few nits and comments; I am still trying to understand the core of the new register allocator.

> JSTests/ChangeLog:8
> +        * stress/tail-call-many-arguments.js: Added.

This is a js test. Do we ever use the new register allocator for js and not wasm code?

> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:57
> +        for (size_t instIndex = 0; instIndex < block->size(); ++instIndex) {

for (Inst& inst : block)
looks possible and simpler

> Source/JavaScriptCore/b3/air/AirGenerate.cpp:81
> +    if (!code.optLevel()) {

Please replace the if (code.optLevel() <= 1) that comes after by if (code.optLevel == 1).
It is a bit confusing to have <= 1, and yet it does not run in -O0.

> Source/JavaScriptCore/b3/air/AirGenerate.cpp:84
> +        // We may still need to do post-allocation lowering. Doing it after both register and

This copy-pasted comment is rather weird considering that
  code.m_generateAndAllocateRegisters = std::make_unique<GenerateAndAllocateRegisters>(code);
  code.m_generateAndAllocateRegisters->prepareForGeneration();
Only appears several lines below.

> Source/JavaScriptCore/b3/air/AirGenerate.cpp:208
> +        context.blockLabels[block] = Box<CCallHelpers::Label>::create();

What is the reasoning behind this change? It does not seem connected to the register allocation

> Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp:1953
> +    ASSERT_UNUSED(compilationMode, compilationMode == CompilationMode::BBQMode);

Why not just eliminate this argument? From a quick search, this function is only called in a single place, and is passed CompilationMode::BBQMode directly.

> Source/WTF/wtf/IndexMap.h:-40
> -    IndexMap()

Nice update of that file.

> Tools/Scripts/run-jsc-stress-tests:488
>  B3O1_OPTIONS = ["--defaultB3OptLevel=1"]

I remain tempted to remove this altogether to compensate for the extra tests added to -O0 (and simplify B3 code), but it is a debate for a different Bugzilla entry.

[Robin Morisset]

Comment on attachment 361716 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=361716&action=review

Just a few more questions. I don't know understand this code well, so they may well be stupid.

> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:209
> +            if (!m_availableRegs[bank].contains(reg))

Why not iterate directly on m_availableRegs?

> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:268
> +    m_allowedRegisters = RegisterSet();

I don't understand the role of this m_allowedRegisters field.
Nothing ever seems to be removed from it, so when is it anything but all registers?

> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:298
> +    // FIXME: We neither use the disassembler nor the CodeOriginMap.

There are tons of references to 'disassembler' in the code below. Is this comment still up-to-date? Does it refer to something else?

> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:324
> +            defaultCurrentAllocation[tmp.reg()] = tmp;

Is there a benefit to this over currentAllocationMap[m_code[0]][tmp.reg()] = tmp;
?
If I understand this code correctly it would also allow removing the special case in the loop just above.

> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:353
> +        blockJumps[block].link(m_jit);

Is it normal that we link the jump to the block before giving a label to that block? I would naively have done it just after, but I don't know that part of the codebase.

[Saam Barati]

Comment on attachment 361716 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=361716&action=review

>> JSTests/ChangeLog:8
>> +        * stress/tail-call-many-arguments.js: Added.
> 
> This is a js test. Do we ever use the new register allocator for js and not wasm code?

Yeah. One of our tests runs JS code in O0

>> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:57
>> +        for (size_t instIndex = 0; instIndex < block->size(); ++instIndex) {
> 
> for (Inst& inst : block)
> looks possible and simpler

fixed.

>> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:209
>> +            if (!m_availableRegs[bank].contains(reg))
> 
> Why not iterate directly on m_availableRegs?

m_registers[bank] is sorted in the order that we want to allocate in. However, that said, I believe the only reason for this is to use caller saves before callee saves (so we don't need to spill callee saves). Air O0 just claims to use all callee saves, so this might not be important. For consistency with the other allocators, I'll just stick with things this way.

>> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:268
>> +    m_allowedRegisters = RegisterSet();
> 
> I don't understand the role of this m_allowedRegisters field.
> Nothing ever seems to be removed from it, so when is it anything but all registers?

It doesn't contain things like CallFrameRegister or any pinned registers. We never need to think about these registers. That's the purpose of this field.

>> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:298
>> +    // FIXME: We neither use the disassembler nor the CodeOriginMap.
> 
> There are tons of references to 'disassembler' in the code below. Is this comment still up-to-date? Does it refer to something else?

good point. This should have been removed once I added support for them.

>> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:324
>> +            defaultCurrentAllocation[tmp.reg()] = tmp;
> 
> Is there a benefit to this over currentAllocationMap[m_code[0]][tmp.reg()] = tmp;
> ?
> If I understand this code correctly it would also allow removing the special case in the loop just above.

Good point. I'll clean this up.

>> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:353
>> +        blockJumps[block].link(m_jit);
> 
> Is it normal that we link the jump to the block before giving a label to that block? I would naively have done it just after, but I don't know that part of the codebase.

The way this works is that previous iterations of this loop may have appended to that JumpList saying they want to just *here*. We link all such jumps now. Any such jumps after that will link to this label assigned below. (See the "link" lambda above)

>> Source/JavaScriptCore/b3/air/AirGenerate.cpp:81
>> +    if (!code.optLevel()) {
> 
> Please replace the if (code.optLevel() <= 1) that comes after by if (code.optLevel == 1).
> It is a bit confusing to have <= 1, and yet it does not run in -O0.

fixed.

>> Source/JavaScriptCore/b3/air/AirGenerate.cpp:84
>> +        // We may still need to do post-allocation lowering. Doing it after both register and
> 
> This copy-pasted comment is rather weird considering that
>   code.m_generateAndAllocateRegisters = std::make_unique<GenerateAndAllocateRegisters>(code);
>   code.m_generateAndAllocateRegisters->prepareForGeneration();
> Only appears several lines below.

replaced with a FIXME that the name of this phase is weird.

>> Source/JavaScriptCore/b3/air/AirGenerate.cpp:208
>> +        context.blockLabels[block] = Box<CCallHelpers::Label>::create();
> 
> What is the reasoning behind this change? It does not seem connected to the register allocation

the null check here is useless. This was a fly-by fix.

>> Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp:1953
>> +    ASSERT_UNUSED(compilationMode, compilationMode == CompilationMode::BBQMode);
> 
> Why not just eliminate this argument? From a quick search, this function is only called in a single place, and is passed CompilationMode::BBQMode directly.

Done.

[Saam Barati]

Created attachment 361989 [details]
WIP

I needed to make some changes to get testb3 to run. I'm going to ask for another review soon.

[Saam Barati]

Created attachment 361990 [details]
WIP

[Saam Barati]

Created attachment 361991 [details]
WIP

[EWS Watchlist]

Comment on attachment 361991 [details]
WIP

Attachment 361991 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11144022

New failing tests:
inspector/css/modify-inline-style.html

[EWS Watchlist]

Created attachment 362002 [details]
Archive of layout-test-results from ews105 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 361991 [details]
WIP

Attachment 361991 [details] did not pass jsc-ews (mac):
Output: https://webkit-queues.webkit.org/results/11144127

New failing tests:
microbenchmarks/super-get-by-id-with-this-polymorphic.js.ftl-no-cjit-b3o0
stress/v8-raytrace-strict.js.ftl-no-cjit-b3o0
microbenchmarks/polyvariant-monomorphic-get-by-id.js.ftl-no-cjit-b3o0
microbenchmarks/get-by-val-with-string-self-or-proto.js.ftl-no-cjit-b3o0
microbenchmarks/array-prototype-map.js.ftl-no-cjit-b3o0
microbenchmarks/array-prototype-forEach.js.ftl-no-cjit-b3o0
stress/phantom-spread-osr-exit.js.ftl-no-cjit-b3o0
stress/spread-multi-layers.js.ftl-no-cjit-b3o0
stress/class-syntax-derived-default-constructor.js.ftl-no-cjit-b3o0
stress/regress-182419.js.ftl-no-cjit-b3o0
microbenchmarks/super-get-by-val-with-this-polymorphic.js.ftl-no-cjit-b3o0
microbenchmarks/array-prototype-reduce.js.ftl-no-cjit-b3o0
microbenchmarks/deltablue-varargs.js.ftl-no-cjit-b3o0
stress/heap-allocator-allocates-incorrect-size-for-activation.js.ftl-no-cjit-b3o0
microbenchmarks/super-get-by-val-with-this-monomorphic.js.ftl-no-cjit-b3o0
microbenchmarks/array-prototype-some.js.ftl-no-cjit-b3o0
stress/phantom-spread-forward-varargs.js.ftl-no-cjit-b3o0
stress/get-argument-by-val-safe-in-inlined-varargs-call-out-of-bounds.js.ftl-no-cjit-b3o0
stress/spread-call-convert-to-static-call.js.ftl-no-cjit-b3o0
stress/load-varargs-on-new-array-with-spread-convert-to-static-loads.js.ftl-no-cjit-b3o0
stress/sampling-profiler-basic.js.ftl-no-cjit-b3o0
stress/dont-validate-stack-offset-in-b3-because-it-might-be-guarded-by-control-flow.js.ftl-no-cjit-b3o0
stress/forward-varargs-double-new-array-buffer.js.ftl-no-cjit-b3o0
microbenchmarks/int-or-other-sub.js.ftl-no-cjit-b3o0
microbenchmarks/int-or-other-sub-then-get-by-val.js.ftl-no-cjit-b3o0
stress/rest-parameter-inlined.js.ftl-no-cjit-b3o0
v8-v6/v8-raytrace.js.ftl-no-cjit-b3o0
stress/call-apply-exponential-bytecode-size.js.ftl-no-cjit-b3o0
microbenchmarks/fold-get-by-id-to-multi-get-by-offset.js.ftl-no-cjit-b3o0
microbenchmarks/get-by-id-proto-or-self.js.ftl-no-cjit-b3o0
stress/tail-call-varargs-no-stack-overflow.js.ftl-no-cjit-b3o0
microbenchmarks/get-by-id-self-or-proto.js.ftl-no-cjit-b3o0
stress/spread-escapes-but-new-array-buffer-does-not-double.js.ftl-no-cjit-b3o0
stress/arguments-interference-cfg.js.ftl-no-cjit-b3o0
microbenchmarks/eval-not-eval-compute-args.js.ftl-no-cjit-b3o0
microbenchmarks/call-spread-apply.js.ftl-no-cjit-b3o0
stress/phantom-new-array-buffer-forward-varargs.js.ftl-no-cjit-b3o0
stress/spread-escapes-but-new-array-buffer-does-not.js.ftl-no-cjit-b3o0
microbenchmarks/array-prototype-reduceRight.js.ftl-no-cjit-b3o0
microbenchmarks/v8-raytrace-with-empty-try-catch.js.ftl-no-cjit-b3o0
stress/model-effects-properly-of-spread-over-phantom-create-rest.js.ftl-no-cjit-b3o0
microbenchmarks/int-or-other-mul-then-get-by-val.js.ftl-no-cjit-b3o0
stress/get-my-argument-by-val-for-inlined-escaped-arguments.js.ftl-no-cjit-b3o0
v8-v6/v8-deltablue.js.ftl-no-cjit-b3o0
microbenchmarks/delta-blue-try-catch.js.ftl-no-cjit-b3o0
v8-v6/v8-splay.js.ftl-no-cjit-b3o0
stress/rest-parameter-various-types.js.ftl-no-cjit-b3o0
stress/arith-nodes-abstract-interpreter-untypeduse.js.ftl-no-cjit-b3o0
stress/phantom-new-array-buffer-forward-varargs2.js.ftl-no-cjit-b3o0
microbenchmarks/array-prototype-every.js.ftl-no-cjit-b3o0
microbenchmarks/int-or-other-div-then-get-by-val.js.ftl-no-cjit-b3o0
stress/spread-escapes-but-create-rest-does-not.js.ftl-no-cjit-b3o0
stress/spread-capture-rest.js.ftl-no-cjit-b3o0
microbenchmarks/v8-raytrace-with-try-catch.js.ftl-no-cjit-b3o0
stress/regress-179140.js.ftl-no-cjit-b3o0
sunspider-1.0/controlflow-recursive.js.ftl-no-cjit-b3o0
microbenchmarks/super-get-by-id-with-this-monomorphic.js.ftl-no-cjit-b3o0
stress/spread-non-varargs.js.ftl-no-cjit-b3o0
stress/phantom-new-array-with-spread-osr-exit.js.ftl-no-cjit-b3o0
v8-v6/v8-earley-boyer.js.ftl-no-cjit-b3o0
sunspider-1.0/access-binary-trees.js.ftl-no-cjit-b3o0
v8-v6/v8-richards.js.ftl-no-cjit-b3o0
stress/get-argument-by-val-in-inlined-varargs-call-out-of-bounds.js.ftl-no-cjit-b3o0
microbenchmarks/fold-get-by-id-to-multi-get-by-offset-rare-int.js.ftl-no-cjit-b3o0
stress/put-direct-index-broken-2.js.ftl-no-cjit-b3o0
microbenchmarks/get-by-val-with-string-proto-or-self.js.ftl-no-cjit-b3o0
apiTests

[Saam Barati]

Created attachment 362055 [details]
WIP

I'm actually going to separate out some of my changes to testb3 into a different patch so I can land this with Yusuke's review.

[Saam Barati]

Created attachment 362098 [details]
patch for landing

[WebKit Commit Bot]

Comment on attachment 362098 [details]
patch for landing

Clearing flags on attachment: 362098

Committed r241579: <https://trac.webkit.org/changeset/241579>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/48103598>