[jsc-dev] [webkit-qt] Release assert in JIT on ARM

[Yusuke SUZUKI]

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 :)

On Fri, Aug 19, 2016 at 10:34 PM, Yusuke SUZUKI <utatane.tea at 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 <annulen at yandex.ru>
> wrote:
>
>>
>>
>> 19.08.2016, 20:43, "Konstantin Tokarev" <annulen at yandex.ru>:
>> > 19.08.2016, 18:34, "Andrew Webster" <awebster at 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 at lists.webkit.org
>> https://lists.webkit.org/mailman/listinfo/webkit-qt
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/jsc-dev/attachments/20160823/48ad43e9/attachment.html>