[webkit-changes] [180317] trunk/Source/JavaScriptCore
Filip Pizlo
fpizlo at apple.com
Wed Feb 18 16:42:50 PST 2015
Elaborating a lot:
Trunk doesn’t have the bug that r180247 fixed, since trunk already has the fix to run FTLCapabilities late. The two fixes - the fail-compile fix in r180247 and the earlier capabilities fix - have perfect overlap in terms of the known crashes that they fix. The two radars should really be dup’d, since the root cause in both is failure to run FTLCapabilities late.
So, we have two fixes that functionally do the same thing: they cause the FTL to quietly give up if it sees IR it cannot handle, which is what it should have been doing all along.
The best argument in favor of the capabilities fix is that FTLCapabilities should accurately tell us what the FTL can handle; we need this to decide whether to put profiling into DFG-compiled code anyway. Running FTLCapabilities late and using that to decide whether to run the FTL backend, but then having the FTL backend assert if it sees bad code, is a good way of ensuring that we keep FTLCapabilities in sync. FWIW, we have done a good job of keeping it in sync and that wasn’t the reason for the crashes; the bug was that we weren’t running it late enough. Running it late means we take an already known-good mechanism for addressing this issue and simply run it at the right time.
The best argument in favor of the fail-compile fix is that it doesn’t require additional perf validation. Previously, FTLCapabilities didn’t have to include an “ok” case for nodes that only arise during SSA lowering, because we erroneously only ran it before SSA lowering. So just adding code to run FTLCapabilities late risks regressing performance. Michael encountered such a case when writing the original capabilities fix patch. We’ve already validated (and fixed) performance on trunk, but we would have to do it again on branches. That seems like more effort than taking the fail-compile fix on branches.
Therefore, the fail-compile fix is most appropriate for branches: we know that it fixes both bugs and we know that it doesn’t require perf validation.
The capabilities fix is correct for trunk.
The remaining question is then: should we have a fail-compile mechanism on trunk? I think this depends on what the fail-compile mechanism looks like. I don’t like the fail-compile fix in r180247 for three reasons: (1) it’s not comprehensive enough, (2) it is a maintenance problem, and (3) it causes an immediate loss of test coverage.
1) The fail-compile fix in r180247 only covers assertions in the FTL lowering backend. In the past, the compiler assertions that caused the most grief have occurred in DFG IR phases rather than the FTL lowering backend (the PutLocalSinking bug from earlier in Gala, the LivenessAnalysis bug from early Syrah, etc). If we’re going to have fail-compile machinery then we should put some thought into making it comprehensive. I’m skeptical that r180247 could be a first step towards a comprehensive solution since the comprehensive solution would hook directly into DFG_CRASH/ASSERT instead of adding a third macro.
2) The fail-compile fix in r180247 relies on having all code paths reachable from a LOWERING_FAILED do additional checks to see if the m_loweringSucceeded flag has been cleared. If it has, then those code paths must also bail early. We know from past experience in the DFG backend that having such a flag causes bugtail. The DFG originally had exactly such a flag, and we had a long tail of bugs where some code path would fail to check this flag and would perform some action that isn’t valid to perform if we had failed an assertion. Most likely, the LOWERING_FAILED machinery as implemented in r180247 would work for a while but would eventually stop working because some code path would get introduced that sits on the failure path but cannot cope with failure. We could alleviate this by either having some other way of diverting control flow in the compiler that doesn’t involve a lot of checks of this boolean, or we could alleviate it by also having tests of the failure code path. In any case, this fail-compile machinery is not free and it will have non-trivial consequences for compiler maintenance. As implemented in r180247, there is nothing to ensure it doesn’t regress; you could say it violates our usual principle that new functionality needs new tests.
3) The immediate consequence of r180247 is not to fix any trunk bugs - the relevant bugs were already fixed by the capabilities fix - but to disable the test coverage we get from crash reports from various kinds of release builds.
Because of the costs associated with maintaining the fail-compile mechanism, I think we should make sure that the risk-reward is right. It’s not right in the current fix because it has non-trivial maintenance cost and reduces test coverage, and the only reward is preventing those crashes in FTLLower that we have already fixed. Future compiler crashes are much more likely to happen in DFG IR phases. Just consider the lines of code involved - DFG IR phases is 70,000 LoC and the lowering code is only 7,000 LoC.
I’m not sure what a robust fail-compile system would look like. Our compiler has methods that call other methods, and often the interesting assertions are deep in the stack. A fail-compile mechanism would have to be able to safely pop stack. In r180247 this is achieved by manually inserting checks of m_loweringSucceeded. I don’t believe that this is scalable, and I wouldn’t agree with any fail-compile proposal that requires stringing “m_okToKeepCompiling” checks throughout the compiler. Because of the difficulty of getting this right, I suggest that we don’t do it unless we can convince ourselves that the mechanism will actually catch some future bug. Putting in fail-compile mechanisms that only cover assertions that we have already fixed isn’t really a good plan: it won’t prevent future crashes, it incurs a maintenance overhead, and it reduces the feedback we get from crash reports.
-Filip
> On Feb 18, 2015, at 4:12 PM, Michael Saboff <msaboff at apple.com> wrote:
>
> Phil is adamant that we should eliminate this from trunk. He believes it is the best solution for avoiding the crash in <rdar://problem/17198412 <rdar://problem/17198412>> CrashTracer: DFG_CRASH beneath JSC::FTL::LowerDFGToLLVM::compileNode, and thinks it is less risky than the fix for <rdar://problem/19187718 <rdar://problem/19187718>> Crash in JSC::FTL::LowerDFGToLLVM::compileCompareStrictEq (139398).
>
> He believes that we lose information by not crashing in internal release builds during our testing. He prefers that we have a comprehensive fail-the-compile-but continue solution for both the DFG and FTL.
>
> We haven’t committed it to the branch, but I suggested that we only commit this change and not the capabilities check landed for <rdar://problem/19187718 <rdar://problem/19187718>> to the branch. I updated rdar://problem/17198412 <rdar://problem/17198412> and its clone rdar://problem/19877186 <rdar://problem/19877186> accordingly. It doesn’t look like rdar://problem/19187718 <rdar://problem/19187718> had been cloned for the branch.
>
> - Michael
>
>> On Feb 18, 2015, at 3:54 PM, Geoffrey Garen <ggaren at apple.com <mailto:ggaren at apple.com>> wrote:
>>
>> If there’s something incorrect or incomplete about this change, and/or if it’s gone from TOT, I don’t agree that we should commit it to the branch.
>>
>> Have we committed the change to the branch?
>>
>> Is there still a Radar proposing committing the change to the branch?
>>
>> Geoff
>>
>>> On Feb 18, 2015, at 3:52 PM, msaboff at apple.com <mailto:msaboff at apple.com> wrote:
>>>
>>> Revision
>>> 180317 <http://trac.webkit.org/projects/webkit/changeset/180317>Author
>>> msaboff at apple.com <mailto:msaboff at apple.com>Date
>>> 2015-02-18 15:52:16 -0800 (Wed, 18 Feb 2015)
>>> Log Message
>>>
>>> Rollout r180247 <http://trac.webkit.org/projects/webkit/changeset/180247> & r180249 <http://trac.webkit.org/projects/webkit/changeset/180249> from trunk
>>> https://bugs.webkit.org/show_bug.cgi?id=141773 <https://bugs.webkit.org/show_bug.cgi?id=141773>
>>>
>>> Reviewed by Filip Pizlo.
>>>
>>> Theses changes makes sense to fix the crash reported in https://bugs.webkit.org/show_bug.cgi?id=141730 <https://bugs.webkit.org/show_bug.cgi?id=141730>
>>> only for branches. The change to fail the FTL compile but continue running is not comprehensive
>>> enough for general use on trunk.
>>>
>>> * dfg/DFGPlan.cpp:
>>> (JSC::DFG::Plan::compileInThreadImpl):
>>> * ftl/FTLLowerDFGToLLVM.cpp:
>>> (JSC::FTL::LowerDFGToLLVM::LowerDFGToLLVM):
>>> (JSC::FTL::LowerDFGToLLVM::lower):
>>> (JSC::FTL::LowerDFGToLLVM::createPhiVariables):
>>> (JSC::FTL::LowerDFGToLLVM::compileNode):
>>> (JSC::FTL::LowerDFGToLLVM::compileUpsilon):
>>> (JSC::FTL::LowerDFGToLLVM::compilePhi):
>>> (JSC::FTL::LowerDFGToLLVM::compileDoubleRep):
>>> (JSC::FTL::LowerDFGToLLVM::compileValueRep):
>>> (JSC::FTL::LowerDFGToLLVM::compileValueToInt32):
>>> (JSC::FTL::LowerDFGToLLVM::compilePutLocal):
>>> (JSC::FTL::LowerDFGToLLVM::compileArithAddOrSub):
>>> (JSC::FTL::LowerDFGToLLVM::compileArithMul):
>>> (JSC::FTL::LowerDFGToLLVM::compileArithDiv):
>>> (JSC::FTL::LowerDFGToLLVM::compileArithMod):
>>> (JSC::FTL::LowerDFGToLLVM::compileArithMinOrMax):
>>> (JSC::FTL::LowerDFGToLLVM::compileArithAbs):
>>> (JSC::FTL::LowerDFGToLLVM::compileArithNegate):
>>> (JSC::FTL::LowerDFGToLLVM::compileArrayifyToStructure):
>>> (JSC::FTL::LowerDFGToLLVM::compileGetById):
>>> (JSC::FTL::LowerDFGToLLVM::compileGetMyArgumentByVal):
>>> (JSC::FTL::LowerDFGToLLVM::compileGetArrayLength):
>>> (JSC::FTL::LowerDFGToLLVM::compileGetByVal):
>>> (JSC::FTL::LowerDFGToLLVM::compilePutByVal):
>>> (JSC::FTL::LowerDFGToLLVM::compileArrayPush):
>>> (JSC::FTL::LowerDFGToLLVM::compileArrayPop):
>>> (JSC::FTL::LowerDFGToLLVM::compileNewArray):
>>> (JSC::FTL::LowerDFGToLLVM::compileToString):
>>> (JSC::FTL::LowerDFGToLLVM::compileMakeRope):
>>> (JSC::FTL::LowerDFGToLLVM::compileCompareEq):
>>> (JSC::FTL::LowerDFGToLLVM::compileCompareStrictEq):
>>> (JSC::FTL::LowerDFGToLLVM::compileSwitch):
>>> (JSC::FTL::LowerDFGToLLVM::compare):
>>> (JSC::FTL::LowerDFGToLLVM::boolify):
>>> (JSC::FTL::LowerDFGToLLVM::opposite):
>>> (JSC::FTL::LowerDFGToLLVM::lowJSValue):
>>> (JSC::FTL::LowerDFGToLLVM::speculate):
>>> (JSC::FTL::LowerDFGToLLVM::isArrayType):
>>> (JSC::FTL::LowerDFGToLLVM::exitValueForAvailability):
>>> (JSC::FTL::LowerDFGToLLVM::exitValueForNode):
>>> (JSC::FTL::LowerDFGToLLVM::setInt52):
>>> (JSC::FTL::lowerDFGToLLVM):
>>> (JSC::FTL::LowerDFGToLLVM::loweringFailed): Deleted.
>>> * ftl/FTLLowerDFGToLLVM.h:
>>> Modified Paths
>>>
>>> trunk/Source/JavaScriptCore/ChangeLog <x-msg://9/#trunkSourceJavaScriptCoreChangeLog>
>>> trunk/Source/JavaScriptCore/dfg/DFGPlan.cpp <x-msg://9/#trunkSourceJavaScriptCoredfgDFGPlancpp>
>>> trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp <x-msg://9/#trunkSourceJavaScriptCoreftlFTLLowerDFGToLLVMcpp>
>>> trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.h <x-msg://9/#trunkSourceJavaScriptCoreftlFTLLowerDFGToLLVMh>
>>> Diff
>>>
>>> <>Modified: trunk/Source/JavaScriptCore/ChangeLog (180316 => 180317)
>>>
>>> --- trunk/Source/JavaScriptCore/ChangeLog 2015-02-18 23:50:12 UTC (rev 180316)
>>> +++ trunk/Source/JavaScriptCore/ChangeLog 2015-02-18 23:52:16 UTC (rev 180317)
>>> @@ -1,3 +1,61 @@
>>> +2015-02-18 Michael Saboff <msaboff at apple.com <mailto:msaboff at apple.com>>
>>> +
>>> + Rollout r180247 & r180249 from trunk
>>> + https://bugs.webkit.org/show_bug.cgi?id=141773 <https://bugs.webkit.org/show_bug.cgi?id=141773>
>>> +
>>> + Reviewed by Filip Pizlo.
>>> +
>>> + Theses changes makes sense to fix the crash reported in https://bugs.webkit.org/show_bug.cgi?id=141730 <https://bugs.webkit.org/show_bug.cgi?id=141730>
>>> + only for branches. The change to fail the FTL compile but continue running is not comprehensive
>>> + enough for general use on trunk.
>>> +
>>> + * dfg/DFGPlan.cpp:
>>> + (JSC::DFG::Plan::compileInThreadImpl):
>>> + * ftl/FTLLowerDFGToLLVM.cpp:
>>> + (JSC::FTL::LowerDFGToLLVM::LowerDFGToLLVM):
>>> + (JSC::FTL::LowerDFGToLLVM::lower):
>>> + (JSC::FTL::LowerDFGToLLVM::createPhiVariables):
>>> + (JSC::FTL::LowerDFGToLLVM::compileNode):
>>> + (JSC::FTL::LowerDFGToLLVM::compileUpsilon):
>>> + (JSC::FTL::LowerDFGToLLVM::compilePhi):
>>> + (JSC::FTL::LowerDFGToLLVM::compileDoubleRep):
>>> + (JSC::FTL::LowerDFGToLLVM::compileValueRep):
>>> + (JSC::FTL::LowerDFGToLLVM::compileValueToInt32):
>>> + (JSC::FTL::LowerDFGToLLVM::compilePutLocal):
>>> + (JSC::FTL::LowerDFGToLLVM::compileArithAddOrSub):
>>> + (JSC::FTL::LowerDFGToLLVM::compileArithMul):
>>> + (JSC::FTL::LowerDFGToLLVM::compileArithDiv):
>>> + (JSC::FTL::LowerDFGToLLVM::compileArithMod):
>>> + (JSC::FTL::LowerDFGToLLVM::compileArithMinOrMax):
>>> + (JSC::FTL::LowerDFGToLLVM::compileArithAbs):
>>> + (JSC::FTL::LowerDFGToLLVM::compileArithNegate):
>>> + (JSC::FTL::LowerDFGToLLVM::compileArrayifyToStructure):
>>> + (JSC::FTL::LowerDFGToLLVM::compileGetById):
>>> + (JSC::FTL::LowerDFGToLLVM::compileGetMyArgumentByVal):
>>> + (JSC::FTL::LowerDFGToLLVM::compileGetArrayLength):
>>> + (JSC::FTL::LowerDFGToLLVM::compileGetByVal):
>>> + (JSC::FTL::LowerDFGToLLVM::compilePutByVal):
>>> + (JSC::FTL::LowerDFGToLLVM::compileArrayPush):
>>> + (JSC::FTL::LowerDFGToLLVM::compileArrayPop):
>>> + (JSC::FTL::LowerDFGToLLVM::compileNewArray):
>>> + (JSC::FTL::LowerDFGToLLVM::compileToString):
>>> + (JSC::FTL::LowerDFGToLLVM::compileMakeRope):
>>> + (JSC::FTL::LowerDFGToLLVM::compileCompareEq):
>>> + (JSC::FTL::LowerDFGToLLVM::compileCompareStrictEq):
>>> + (JSC::FTL::LowerDFGToLLVM::compileSwitch):
>>> + (JSC::FTL::LowerDFGToLLVM::compare):
>>> + (JSC::FTL::LowerDFGToLLVM::boolify):
>>> + (JSC::FTL::LowerDFGToLLVM::opposite):
>>> + (JSC::FTL::LowerDFGToLLVM::lowJSValue):
>>> + (JSC::FTL::LowerDFGToLLVM::speculate):
>>> + (JSC::FTL::LowerDFGToLLVM::isArrayType):
>>> + (JSC::FTL::LowerDFGToLLVM::exitValueForAvailability):
>>> + (JSC::FTL::LowerDFGToLLVM::exitValueForNode):
>>> + (JSC::FTL::LowerDFGToLLVM::setInt52):
>>> + (JSC::FTL::lowerDFGToLLVM):
>>> + (JSC::FTL::LowerDFGToLLVM::loweringFailed): Deleted.
>>> + * ftl/FTLLowerDFGToLLVM.h:
>>> +
>>> 2015-02-18 Filip Pizlo <fpizlo at apple.com <mailto:fpizlo at apple.com>>
>>>
>>> DFG should really support varargs
>>> <>Modified: trunk/Source/JavaScriptCore/dfg/DFGPlan.cpp (180316 => 180317)
>>>
>>> --- trunk/Source/JavaScriptCore/dfg/DFGPlan.cpp 2015-02-18 23:50:12 UTC (rev 180316)
>>> +++ trunk/Source/JavaScriptCore/dfg/DFGPlan.cpp 2015-02-18 23:52:16 UTC (rev 180317)
>>> @@ -386,10 +386,7 @@
>>> }
>>>
>>> FTL::State state(dfg);
>>> - if (!FTL::lowerDFGToLLVM(state)) {
>>> - FTL::fail(state);
>>> - return FTLPath;
>>> - }
>>> + FTL::lowerDFGToLLVM(state);
>>>
>>> if (reportCompileTimes())
>>> m_timeBeforeFTL = monotonicallyIncreasingTime();
>>> <>Modified: trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp (180316 => 180317)
>>>
>>> --- trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp 2015-02-18 23:50:12 UTC (rev 180316)
>>> +++ trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp 2015-02-18 23:52:16 UTC (rev 180317)
>>> @@ -93,7 +93,6 @@
>>> LowerDFGToLLVM(State& state)
>>> : m_graph(state.graph)
>>> , m_ftlState(state)
>>> - , m_loweringSucceeded(true)
>>> , m_heaps(state.context)
>>> , m_out(state.context)
>>> , m_state(state.graph)
>>> @@ -103,12 +102,8 @@
>>> , m_tbaaStructKind(mdKindID(state.context, "tbaa.struct"))
>>> {
>>> }
>>> -
>>> -
>>> -#define LOWERING_FAILED(node, reason) \
>>> - loweringFailed((node), __FILE__, __LINE__, WTF_PRETTY_FUNCTION, (reason));
>>> -
>>> - bool lower()
>>> +
>>> + void lower()
>>> {
>>> CString name;
>>> if (verboseCompilationEnabled()) {
>>> @@ -280,22 +275,14 @@
>>> case FlushedJSValue:
>>> break;
>>> default:
>>> - LOWERING_FAILED(node, "Bad flush format for argument");
>>> + DFG_CRASH(m_graph, node, "Bad flush format for argument");
>>> break;
>>> }
>>> }
>>> -
>>> - if (!m_loweringSucceeded)
>>> - return m_loweringSucceeded;
>>> -
>>> m_out.jump(lowBlock(m_graph.block(0)));
>>>
>>> - for (BasicBlock* block : preOrder) {
>>> + for (BasicBlock* block : preOrder)
>>> compileBlock(block);
>>> -
>>> - if (!m_loweringSucceeded)
>>> - return m_loweringSucceeded;
>>> - }
>>>
>>> if (Options::dumpLLVMIR())
>>> dumpModule(m_ftlState.module);
>>> @@ -304,8 +291,6 @@
>>> m_ftlState.dumpState("after lowering");
>>> if (validationEnabled())
>>> verifyModule(m_ftlState.module);
>>> -
>>> - return m_loweringSucceeded;
>>> }
>>>
>>> private:
>>> @@ -338,8 +323,8 @@
>>> type = m_out.int64;
>>> break;
>>> default:
>>> - LOWERING_FAILED(node, "Bad Phi node result type");
>>> - return;
>>> + DFG_CRASH(m_graph, node, "Bad Phi node result type");
>>> + break;
>>> }
>>> m_phis.add(node, buildAlloca(m_out.m_builder, type));
>>> }
>>> @@ -843,13 +828,10 @@
>>> case KillLocal:
>>> break;
>>> default:
>>> - LOWERING_FAILED(m_node, "Unrecognized node in FTL backend");
>>> + DFG_CRASH(m_graph, m_node, "Unrecognized node in FTL backend");
>>> break;
>>> }
>>>
>>> - if (!m_loweringSucceeded)
>>> - return false;
>>> -
>>> if (!m_state.isValid()) {
>>> safelyInvalidateAfterTermination();
>>> return false;
>>> @@ -885,7 +867,7 @@
>>> m_out.set(lowJSValue(m_node->child1()), destination);
>>> break;
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad use kind");
>>> + DFG_CRASH(m_graph, m_node, "Bad use kind");
>>> break;
>>> }
>>> }
>>> @@ -911,7 +893,7 @@
>>> setJSValue(m_out.get(source));
>>> break;
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad use kind");
>>> + DFG_CRASH(m_graph, m_node, "Bad use kind");
>>> break;
>>> }
>>> }
>>> @@ -949,7 +931,7 @@
>>> }
>>>
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad use kind");
>>> + DFG_CRASH(m_graph, m_node, "Bad use kind");
>>> }
>>> }
>>>
>>> @@ -974,7 +956,7 @@
>>> }
>>>
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad use kind");
>>> + DFG_CRASH(m_graph, m_node, "Bad use kind");
>>> }
>>> }
>>>
>>> @@ -1037,7 +1019,7 @@
>>> }
>>>
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad use kind");
>>> + DFG_CRASH(m_graph, m_node, "Bad use kind");
>>> break;
>>> }
>>> }
>>> @@ -1144,7 +1126,7 @@
>>> }
>>>
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad flush format");
>>> + DFG_CRASH(m_graph, m_node, "Bad flush format");
>>> break;
>>> }
>>> }
>>> @@ -1304,7 +1286,7 @@
>>> }
>>>
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad use kind");
>>> + DFG_CRASH(m_graph, m_node, "Bad use kind");
>>> break;
>>> }
>>> }
>>> @@ -1378,7 +1360,7 @@
>>> }
>>>
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad use kind");
>>> + DFG_CRASH(m_graph, m_node, "Bad use kind");
>>> break;
>>> }
>>> }
>>> @@ -1481,7 +1463,7 @@
>>> }
>>>
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad use kind");
>>> + DFG_CRASH(m_graph, m_node, "Bad use kind");
>>> break;
>>> }
>>> }
>>> @@ -1579,7 +1561,7 @@
>>> }
>>>
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad use kind");
>>> + DFG_CRASH(m_graph, m_node, "Bad use kind");
>>> break;
>>> }
>>> }
>>> @@ -1630,7 +1612,7 @@
>>> }
>>>
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad use kind");
>>> + DFG_CRASH(m_graph, m_node, "Bad use kind");
>>> break;
>>> }
>>> }
>>> @@ -1656,7 +1638,7 @@
>>> }
>>>
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad use kind");
>>> + DFG_CRASH(m_graph, m_node, "Bad use kind");
>>> break;
>>> }
>>> }
>>> @@ -1781,7 +1763,7 @@
>>> }
>>>
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad use kind");
>>> + DFG_CRASH(m_graph, m_node, "Bad use kind");
>>> break;
>>> }
>>> }
>>> @@ -1919,8 +1901,8 @@
>>> vmCall(m_out.operation(operationEnsureArrayStorage), m_callFrame, cell);
>>> break;
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad array type");
>>> - return;
>>> + DFG_CRASH(m_graph, m_node, "Bad array type");
>>> + break;
>>> }
>>>
>>> structureID = m_out.load32(cell, m_heaps.JSCell_structureID);
>>> @@ -1988,7 +1970,7 @@
>>> }
>>>
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad use kind");
>>> + DFG_CRASH(m_graph, m_node, "Bad use kind");
>>> return;
>>> }
>>> }
>>> @@ -2160,8 +2142,7 @@
>>> // FIXME: FTL should support activations.
>>> // https://bugs.webkit.org/show_bug.cgi?id=129576 <https://bugs.webkit.org/show_bug.cgi?id=129576>
>>>
>>> - LOWERING_FAILED(m_node, "Unimplemented");
>>> - return;
>>> + DFG_CRASH(m_graph, m_node, "Unimplemented");
>>> }
>>>
>>> TypedPointer base;
>>> @@ -2207,7 +2188,7 @@
>>> return;
>>> }
>>>
>>> - LOWERING_FAILED(m_node, "Bad array type");
>>> + DFG_CRASH(m_graph, m_node, "Bad array type");
>>> return;
>>> }
>>> }
>>> @@ -2357,8 +2338,7 @@
>>> result = m_out.load32(pointer);
>>> break;
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad element size");
>>> - return;
>>> + DFG_CRASH(m_graph, m_node, "Bad element size");
>>> }
>>>
>>> if (elementSize(type) < 4) {
>>> @@ -2402,15 +2382,14 @@
>>> result = m_out.loadDouble(pointer);
>>> break;
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad typed array type");
>>> - return;
>>> + DFG_CRASH(m_graph, m_node, "Bad typed array type");
>>> }
>>>
>>> setDouble(result);
>>> return;
>>> }
>>>
>>> - LOWERING_FAILED(m_node, "Bad array type");
>>> + DFG_CRASH(m_graph, m_node, "Bad array type");
>>> return;
>>> } }
>>> }
>>> @@ -2514,10 +2493,9 @@
>>> m_out.storeDouble(value, elementPointer);
>>> break;
>>> }
>>> -
>>> +
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad array type");
>>> - return;
>>> + DFG_CRASH(m_graph, m_node, "Bad array type");
>>> }
>>>
>>> m_out.jump(continuation);
>>> @@ -2610,8 +2588,7 @@
>>> }
>>>
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad use kind");
>>> - return;
>>> + DFG_CRASH(m_graph, m_node, "Bad use kind");
>>> }
>>>
>>> switch (elementSize(type)) {
>>> @@ -2628,8 +2605,7 @@
>>> refType = m_out.ref32;
>>> break;
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad element size");
>>> - return;
>>> + DFG_CRASH(m_graph, m_node, "Bad element size");
>>> }
>>> } else /* !isInt(type) */ {
>>> LValue value = lowDouble(child3);
>>> @@ -2643,8 +2619,7 @@
>>> refType = m_out.refDouble;
>>> break;
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad typed array type");
>>> - return;
>>> + DFG_CRASH(m_graph, m_node, "Bad typed array type");
>>> }
>>> }
>>>
>>> @@ -2668,8 +2643,8 @@
>>> return;
>>> }
>>>
>>> - LOWERING_FAILED(m_node, "Bad array type");
>>> - return;
>>> + DFG_CRASH(m_graph, m_node, "Bad array type");
>>> + break;
>>> }
>>> }
>>>
>>> @@ -2740,7 +2715,7 @@
>>> }
>>>
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad array type");
>>> + DFG_CRASH(m_graph, m_node, "Bad array type");
>>> return;
>>> }
>>> }
>>> @@ -2798,7 +2773,7 @@
>>> }
>>>
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad array type");
>>> + DFG_CRASH(m_graph, m_node, "Bad array type");
>>> return;
>>> }
>>> }
>>> @@ -2835,8 +2810,8 @@
>>> switch (m_node->indexingType()) {
>>> case ALL_BLANK_INDEXING_TYPES:
>>> case ALL_UNDECIDED_INDEXING_TYPES:
>>> - LOWERING_FAILED(m_node, "Bad indexing type");
>>> - return;
>>> + DFG_CRASH(m_graph, m_node, "Bad indexing type");
>>> + break;
>>>
>>> case ALL_DOUBLE_INDEXING_TYPES:
>>> m_out.storeDouble(
>>> @@ -2853,8 +2828,8 @@
>>> break;
>>>
>>> default:
>>> - LOWERING_FAILED(m_node, "Corrupt indexing type");
>>> - return;
>>> + DFG_CRASH(m_graph, m_node, "Corrupt indexing type");
>>> + break;
>>> }
>>> }
>>>
>>> @@ -3135,8 +3110,8 @@
>>> }
>>>
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad use kind");
>>> - return;
>>> + DFG_CRASH(m_graph, m_node, "Bad use kind");
>>> + break;
>>> }
>>> }
>>>
>>> @@ -3227,8 +3202,8 @@
>>> m_out.operation(operationMakeRope3), m_callFrame, kids[0], kids[1], kids[2]));
>>> break;
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad number of children");
>>> - return;
>>> + DFG_CRASH(m_graph, m_node, "Bad number of children");
>>> + break;
>>> }
>>> m_out.jump(continuation);
>>>
>>> @@ -3648,8 +3623,8 @@
>>> nonSpeculativeCompare(LLVMIntEQ, operationCompareEq);
>>> return;
>>> }
>>> -
>>> - LOWERING_FAILED(m_node, "Bad use kinds");
>>> +
>>> + DFG_CRASH(m_graph, m_node, "Bad use kinds");
>>> }
>>>
>>> void compileCompareEqConstant()
>>> @@ -3742,7 +3717,7 @@
>>> return;
>>> }
>>>
>>> - LOWERING_FAILED(m_node, "Bad use kinds");
>>> + DFG_CRASH(m_graph, m_node, "Bad use kinds");
>>> }
>>>
>>> void compileCompareStrictEqConstant()
>>> @@ -4009,8 +3984,8 @@
>>> }
>>>
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad use kind");
>>> - return;
>>> + DFG_CRASH(m_graph, m_node, "Bad use kind");
>>> + break;
>>> }
>>>
>>> m_out.appendTo(switchOnInts, lastNext);
>>> @@ -4055,8 +4030,8 @@
>>> }
>>>
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad use kind");
>>> - return;
>>> + DFG_CRASH(m_graph, m_node, "Bad use kind");
>>> + break;
>>> }
>>>
>>> LBasicBlock lengthIs1 = FTL_NEW_BLOCK(m_out, ("Switch/SwitchChar length is 1"));
>>> @@ -4108,7 +4083,7 @@
>>> }
>>>
>>> case SwitchString: {
>>> - LOWERING_FAILED(m_node, "Unimplemented");
>>> + DFG_CRASH(m_graph, m_node, "Unimplemented");
>>> return;
>>> }
>>>
>>> @@ -4131,7 +4106,7 @@
>>> }
>>>
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad use kind");
>>> + DFG_CRASH(m_graph, m_node, "Bad use kind");
>>> return;
>>> }
>>>
>>> @@ -4139,7 +4114,7 @@
>>> return;
>>> } }
>>>
>>> - LOWERING_FAILED(m_node, "Bad switch kind");
>>> + DFG_CRASH(m_graph, m_node, "Bad switch kind");
>>> }
>>>
>>> void compileReturn()
>>> @@ -5109,7 +5084,7 @@
>>> return;
>>> }
>>>
>>> - LOWERING_FAILED(m_node, "Bad use kinds");
>>> + DFG_CRASH(m_graph, m_node, "Bad use kinds");
>>> }
>>>
>>> void compareEqObjectOrOtherToObject(Edge leftChild, Edge rightChild)
>>> @@ -5422,7 +5397,7 @@
>>> return m_out.phi(m_out.boolean, fastResult, slowResult);
>>> }
>>> default:
>>> - LOWERING_FAILED(m_node, "Bad use kind");
>>> + DFG_CRASH(m_graph, m_node, "Bad use kind");
>>> return 0;
>>> }
>>> }
>>> @@ -5801,7 +5776,7 @@
>>> case StrictInt52:
>>> return Int52;
>>> }
>>> - LOWERING_FAILED(m_node, "Bad use kind");
>>> + DFG_CRASH(m_graph, m_node, "Bad use kind");
>>> return Int52;
>>> }
>>>
>>> @@ -5945,7 +5920,7 @@
>>> return result;
>>> }
>>>
>>> - LOWERING_FAILED(m_node, "Value not defined");
>>> + DFG_CRASH(m_graph, m_node, "Value not defined");
>>> return 0;
>>> }
>>>
>>> @@ -6260,8 +6235,7 @@
>>> speculateMisc(edge);
>>> break;
>>> default:
>>> - LOWERING_FAILED(m_node, "Unsupported speculation use kind");
>>> - return;
>>> + DFG_CRASH(m_graph, m_node, "Unsupported speculation use kind");
>>> }
>>> }
>>>
>>> @@ -6322,7 +6296,7 @@
>>>
>>> switch (arrayMode.arrayClass()) {
>>> case Array::OriginalArray:
>>> - LOWERING_FAILED(m_node, "Unexpected original array");
>>> + DFG_CRASH(m_graph, m_node, "Unexpected original array");
>>> return 0;
>>>
>>> case Array::Array:
>>> @@ -6342,8 +6316,7 @@
>>> m_out.constInt8(arrayMode.shapeMask()));
>>> }
>>>
>>> - LOWERING_FAILED(m_node, "Corrupt array class");
>>> - return 0;
>>> + DFG_CRASH(m_graph, m_node, "Corrupt array class");
>>> }
>>>
>>> default:
>>> @@ -6853,7 +6826,7 @@
>>> return ExitValue::argumentsObjectThatWasNotCreated();
>>> }
>>>
>>> - LOWERING_FAILED(m_node, "Invalid flush format");
>>> + DFG_CRASH(m_graph, m_node, "Invalid flush format");
>>> return ExitValue::dead();
>>> }
>>>
>>> @@ -6926,7 +6899,7 @@
>>> if (isValid(value))
>>> return exitArgument(arguments, ValueFormatDouble, value.value());
>>>
>>> - LOWERING_FAILED(m_node, toCString("Cannot find value for node: ", node).data());
>>> + DFG_CRASH(m_graph, m_node, toCString("Cannot find value for node: ", node).data());
>>> return ExitValue::dead();
>>> }
>>>
>>> @@ -6984,7 +6957,7 @@
>>> return;
>>> }
>>>
>>> - LOWERING_FAILED(m_node, "Corrupt int52 kind");
>>> + DFG_CRASH(m_graph, m_node, "Corrupt int52 kind");
>>> }
>>> void setJSValue(Node* node, LValue value)
>>> {
>>> @@ -7139,20 +7112,6 @@
>>> m_out.unreachable();
>>> }
>>>
>>> - NO_RETURN_DUE_TO_ASSERT void loweringFailed(Node* node, const char* file, int line, const char* function, const char* assertion)
>>> - {
>>> -#ifndef NDEBUG
>>> - m_graph.handleAssertionFailure(node, file, line, function, (assertion));
>>> -#else
>>> - UNUSED_PARAM(node);
>>> - UNUSED_PARAM(file);
>>> - UNUSED_PARAM(line);
>>> - UNUSED_PARAM(function);
>>> - UNUSED_PARAM(assertion);
>>> -#endif
>>> - m_loweringSucceeded = false;
>>> - }
>>> -
>>> AvailabilityMap& availabilityMap() { return m_availabilityCalculator.m_availability; }
>>>
>>> VM& vm() { return m_graph.m_vm; }
>>> @@ -7160,7 +7119,6 @@
>>>
>>> Graph& m_graph;
>>> State& m_ftlState;
>>> - bool m_loweringSucceeded;
>>> AbstractHeapRepository m_heaps;
>>> Output m_out;
>>>
>>> @@ -7208,14 +7166,12 @@
>>> uint32_t m_stackmapIDs;
>>> unsigned m_tbaaKind;
>>> unsigned m_tbaaStructKind;
>>> -
>>> -#undef LOWERING_FAILED
>>> };
>>>
>>> -bool lowerDFGToLLVM(State& state)
>>> +void lowerDFGToLLVM(State& state)
>>> {
>>> LowerDFGToLLVM lowering(state);
>>> - return lowering.lower();
>>> + lowering.lower();
>>> }
>>>
>>> } } // namespace JSC::FTL
>>> <>Modified: trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.h (180316 => 180317)
>>>
>>> --- trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.h 2015-02-18 23:50:12 UTC (rev 180316)
>>> +++ trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.h 2015-02-18 23:52:16 UTC (rev 180317)
>>> @@ -33,7 +33,7 @@
>>>
>>> namespace JSC { namespace FTL {
>>>
>>> -bool lowerDFGToLLVM(State&);
>>> +void lowerDFGToLLVM(State&);
>>>
>>> } } // namespace JSC::FTL
>>>
>>> _______________________________________________
>>> webkit-changes mailing list
>>> webkit-changes at lists.webkit.org <mailto:webkit-changes at lists.webkit.org>
>>> https://lists.webkit.org/mailman/listinfo/webkit-changes <https://lists.webkit.org/mailman/listinfo/webkit-changes>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-changes/attachments/20150218/1b01ba5a/attachment-0001.html>
More information about the webkit-changes
mailing list