For the record, I've finally merged this patch into qtwebkit-stable branch (with correction).
25.08.2016, 16:30, "Andrew Webster" <awebs...@arcx.com>: > Konstantin, > > The patch uses AnyIntUse, which is not defined. MachineIntUse was renamed to > AnyIntUse, see https://bugs.webkit.org/show_bug.cgi?id=156941. > > The corresponding git commit is 39e9c984. I guess you would either need to > grab that commit as well, or rename AnyIntUse back to MachineIntUse. I > renamed back to MachineIntUse for testing and can confirm that it fixes my > problem. I have not yet run the JSC test suite. > > Thanks a lot Yusuke and Konstantin! > > Andrew > ________________________________________ > From: Konstantin Tokarev <annu...@yandex.ru> > Sent: Tuesday, August 23, 2016 2:00 PM > To: Yusuke SUZUKI > Cc: Andrew Webster; webkit-qt@lists.webkit.org; jsc-...@lists.webkit.org > Subject: Re: [webkit-qt] Release assert in JIT on ARM > > 23.08.2016, 20:59, "Yusuke SUZUKI" <utatane....@gmail.com>: >> Fixed. https://trac.webkit.org/changeset/204699 >> >> So, I think Konstantin will update the QtWebKitNG for the next Technology >> Preview. >> Once it is done & released, this issue is fixed :) > > I've prepared a backport of this patch to qtwebkit-stable branch, but haven't > checked it yet: > > https://github.com/annulen/webkit/commit/26abba03efc2013b3937a32c815c0111572f225c > >> On Fri, Aug 19, 2016 at 10:34 PM, Yusuke SUZUKI <utatane....@gmail.com> >> wrote: >>> Nice catch! >>> >>> I've just filed it in https://bugs.webkit.org/show_bug.cgi?id=161029. >>> AnyInt includes int52 representation, that is only allowed in 64bit DFG. >>> (See enableInt52()) >>> >>> On Sat, Aug 20, 2016 at 2:49 AM, Konstantin Tokarev <annu...@yandex.ru> >>> wrote: >>>> 19.08.2016, 20:43, "Konstantin Tokarev" <annu...@yandex.ru>: >>>>> 19.08.2016, 18:34, "Andrew Webster" <awebs...@arcx.com>: >>>>>> This may be a question for webkit-dev, but I thought I'd check here >>>>>> first since I'm using qtwebkit-tp3. >>>>>> >>>>>> On an arm 32-bit platform in SpeculativeJIT::speculate, I occasionally >>>>>> hit the default handler which contains a release assert when using the >>>>>> WebInspector: >>>>>> >>>>>> switch (edge.useKind()) { >>>>>> >>>>>> ... >>>>>> >>>>>> default: >>>>>> RELEASE_ASSERT_NOT_REACHED(); >>>>>> break; >>>>>> } >>>>>> >>>>>> The value of edge.useKind() causing this is MachineIntUse. The case >>>>>> handler for this value has been ifdef'd out on my platform: >>>>>> >>>>>> #if USE(JSVALUE64) >>>>>> case MachineIntUse: >>>>>> speculateMachineInt(edge); >>>>>> break; >>>>>> case DoubleRepMachineIntUse: >>>>>> speculateDoubleRepMachineInt(edge); >>>>>> break; >>>>>> #endif >>>>>> >>>>>> It appears that MachineIntUse is being set in >>>>>> JSC::DFG::FixupPhase::fixupNode when op is ProfileType: >>>>>> >>>>>> if (typeSet->doesTypeConformTo(TypeMachineInt)) { >>>>>> if (node->child1()->shouldSpeculateInt32()) >>>>>> fixEdge<Int32Use>(node->child1()); >>>>>> else >>>>>> fixEdge<MachineIntUse>(node->child1()); >>>>>> node->remove(); >>>>>> } >>>>>> >>>>>> I am not at all familiar with this code, but from other usage of >>>>>> MachineIntUse, I would guess that this should not be used except on a >>>>>> 64-bit platform. Given that, I am not sure if >>>>>> >>>>>> 1. The typeSet should not conform to TypeMachineInt on 32-bit, >>>>>> >>>>>> 2. shouldSpeculateInt32 should always be true on 32-bit, >>>>>> >>>>>> 3. Int32Use should always be used on 32-bit, or >>>>>> >>>>>> 4. Something else. >>>>>> >>>>>> I currently am going with 3: >>>>>> >>>>>> if (typeSet->doesTypeConformTo(TypeMachineInt)) { >>>>>> #if USE(JSVALUE64) >>>>>> if (node->child1()->shouldSpeculateInt32()) >>>>>> #endif >>>>>> fixEdge<Int32Use>(node->child1()); >>>>>> #if USE(JSVALUE64) >>>>>> else >>>>>> fixEdge<MachineIntUse>(node->child1()); >>>>>> #endif >>>>>> >>>>>> } >>>>>> >>>>>> This has solved my immediate problem, but due to my lack of >>>>>> understanding, this solution could be quite flawed. >>>>>> >>>>>> Any help is much appreciated. >>>>> >>>>> Hello, thanks for the interest! >>>>> >>>>> I'm by no means a JSC expert, however from quick analysis it seems to me >>>>> that the correct code would be >>>>> >>>>> #if USE(JSVALUE64) >>>>> if (typeSet->doesTypeConformTo(TypeMachineInt)) { >>>>> if (node->child1()->shouldSpeculateInt32()) >>>>> fixEdge<Int32Use>(node->child1()); >>>>> else >>>>> fixEdge<MachineIntUse>(node->child1()); >>>>> node->remove(); >>>>> } >>>>> #else >>>>> if (typeSet->doesTypeConformTo(TypeMachineInt) && >>>>> node->child1()->shouldSpeculateInt32()) { >>>>> fixEdge<Int32Use>(node->child1()); >>>>> node->remove(); >>>>> } >>>>> #endif >>>>> >>>>> Anyway, I highly recommend you to: >>>>> >>>>> 1. Ask real JSC experts on webkit-dev or jsc-dev >>>>> 2. Run JSC test suite on target (better debug build as well, as it has >>>>> much more ASSERTs) before and after such changes >>>> >>>> Sorry, I forgot to add an explanation: AFAIU, MachineInt is Int32 | Int52 >>>> and on 32-bit platforms we don't speculate about Int52 because it won't >>>> fit in the register anyway, so MachineInt can be only Int32. If we have a >>>> MachineInt which is not inferred to be Int32, we cannot do anything fast >>>> with it and we follow to the next branch TypeNumber | TypeMachineInt. >>>> >>>> -- >>>> Regards, >>>> Konstantin >>>> _______________________________________________ >>>> webkit-qt mailing list >>>> webkit-qt@lists.webkit.org >>>> https://lists.webkit.org/mailman/listinfo/webkit-qt > > -- > Regards, > Konstantin -- Regards, Konstantin _______________________________________________ webkit-qt mailing list webkit-qt@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-qt