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 <[email protected]>
Sent: Tuesday, August 23, 2016 2:00 PM
To: Yusuke SUZUKI
Cc: Andrew Webster; [email protected]; [email protected]
Subject: Re: [webkit-qt] Release assert in JIT on ARM

23.08.2016, 20:59, "Yusuke SUZUKI" <[email protected]>:
> 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 <[email protected]> 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 <[email protected]> 
>> wrote:
>>> 19.08.2016, 20:43, "Konstantin Tokarev" <[email protected]>:
>>>> 19.08.2016, 18:34, "Andrew Webster" <[email protected]>:
>>>>>  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
>>> [email protected]
>>> https://lists.webkit.org/mailman/listinfo/webkit-qt


--
Regards,
Konstantin
_______________________________________________
webkit-qt mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-qt

Reply via email to