Attachment 361616 Details for Bug 194053 – patch

[patch] patch

c-backup.diff (text/plain), 4.13 KB, created by Saam Barati on 2019-02-09 17:03:24 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Saam Barati

Created: 2019-02-09 17:03:24 PST

Size: 4.13 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 241247)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,16 @@
>+2019-02-09  Saam barati  <sbarati@apple.com>
>+
>+        BBQ-Air: Emit better code for switch
>+        https://bugs.webkit.org/show_bug.cgi?id=194053
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Instead of emitting a linear set of jumps for Switch, this patch
>+        makes the BBQ-Air backend emit a binary switch.
>+
>+        * wasm/WasmAirIRGenerator.cpp:
>+        (JSC::Wasm::AirIRGenerator::addSwitch):
>+
> 2019-02-09  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         Unreviewed, Lexer should use isLatin1 implementation in WTF
>Index: Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp
>===================================================================
>--- Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp	(revision 241246)
>+++ Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp	(working copy)
>@@ -39,6 +39,7 @@
> #include "B3PatchpointSpecial.h"
> #include "B3Procedure.h"
> #include "B3ProcedureInlines.h"
>+#include "BinarySwitch.h"
> #include "ScratchRegisterAllocator.h"
> #include "VirtualRegister.h"
> #include "WasmCallingConvention.h"
>@@ -1527,22 +1528,57 @@ auto AirIRGenerator::addSwitch(Expressio
>         unifyValuesWithBlock(expressionStack, targets[i]->resultForBranch());
>     unifyValuesWithBlock(expressionStack, defaultTarget.resultForBranch());
> 
>-    // FIXME: Emit either a jump table or a binary switch here.
>-    // https://bugs.webkit.org/show_bug.cgi?id=194053
>+    ASSERT(condition.type() == Type::I32);
> 
>-    for (size_t i = 0; i < targets.size(); ++i) {
>-        BasicBlock* target = targets[i]->targetBlockForBranch();
>-        BasicBlock* continuation = m_code.addBlock();
>-        auto constant = g64();
>-        append(Move, Arg::bigImm(i), constant);
>-        append(Branch32, Arg::relCond(MacroAssembler::Equal), constant, condition);
>-        m_currentBlock->setSuccessors(target, continuation);
>+    // FIXME: We should consider dynamically switching between a jump table
>+    // and a binary switch depending on the number of successors.
>+    // https://bugs.webkit.org/show_bug.cgi?id=194477
> 
>-        m_currentBlock = continuation;
>-    }
>+    auto& successors = m_currentBlock->successors();
>+    ASSERT(successors.isEmpty());
>+    for (size_t i = 0; i < targets.size(); ++i)
>+        successors.append(targets[i]->targetBlockForBranch());
>+    successors.append(defaultTarget.targetBlockForBranch());
> 
>-    append(Jump);
>-    m_currentBlock->setSuccessors(defaultTarget.targetBlockForBranch());
>+    size_t numTargets = targets.size();
>+
>+    auto* patchpoint = addPatchpoint(B3::Void);
>+    patchpoint->effects = B3::Effects::none();
>+    patchpoint->effects.terminal = true;
>+
>+    patchpoint->setGenerator([=] (CCallHelpers& jit, const B3::StackmapGenerationParams& params) {
>+        Vector<int64_t> cases;
>+        cases.reserveInitialCapacity(numTargets);
>+        for (size_t i = 0; i < numTargets; ++i)
>+            cases.uncheckedAppend(i);
>+
>+        GPRReg valueReg = params[0].gpr();
>+        BinarySwitch binarySwitch(valueReg, cases, BinarySwitch::Int32);
>+
>+        Vector<CCallHelpers::Jump> caseJumps;
>+        caseJumps.resize(numTargets);
>+
>+        while (binarySwitch.advance(jit)) {
>+            unsigned value = binarySwitch.caseValue();
>+            unsigned index = binarySwitch.caseIndex();
>+            ASSERT_UNUSED(value, value == index);
>+            ASSERT(index < numTargets);
>+            caseJumps[index] = jit.jump();
>+        }
>+
>+        CCallHelpers::JumpList fallThrough = binarySwitch.fallThrough();
>+
>+        Vector<Box<CCallHelpers::Label>> successorLabels = params.successorLabels();
>+        ASSERT(successorLabels.size() == caseJumps.size() + 1);
>+
>+        params.addLatePath([=] (CCallHelpers& jit) {
>+            for (size_t i = 0; i < numTargets; ++i)
>+                caseJumps[i].linkTo(*successorLabels[i], &jit);                
>+            fallThrough.linkTo(*successorLabels[numTargets], &jit);
>+        });
>+    });
>+
>+    emitPatchpoint(patchpoint, TypedTmp(), condition);
> 
>     return { };
> }

Flags:

ysuzuki: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 194053: 361615 | 361616