[Webkit-unassigned] [Bug 137020] [CLoop] - Fix CLoop on the 32-bit Big-Endians

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Mar 23 21:35:01 PDT 2015


https://bugs.webkit.org/show_bug.cgi?id=137020

--- Comment #10 from Filip Pizlo <fpizlo at apple.com> ---
(In reply to comment #9)
> Comment on attachment 238528 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=238528&action=review
> 
> > Source/JavaScriptCore/llint/LowLevelInterpreter.asm:540
> > -    loadp Callee[cfr], targetRegister
> > +    if JSVALUE64
> > +        loadp Callee[cfr], targetRegister
> > +    else
> > +        loadp Callee + PayloadOffset[cfr], targetRegister
> > +    end
> 
> I’m thinking that for 64-bit, PayloadOffset will always be 0 anyway.  Hence,
> this change should not be needed.  Is that not the case?

Nope, PayloadOffset is non-zero in 64-bit.  We use it to mean the following in 64-bit: if you stored a JSValue containing an int32, then what is the offset of the int32?

However, this change is really dirty and I object to the approach.  I recommend having a CellOffset defined, whose meaning is: PayloadOffset on 32-bit and 0 on 64-bit.  I believe that I've made a similar comment on a related bug.

> 
> > Source/JavaScriptCore/llint/LowLevelInterpreter.asm:550
> > -    loadp Callee[cfr], targetRegister
> > +    if JSVALUE64
> > +        loadp Callee[cfr], targetRegister
> > +    else
> > +        loadp Callee + PayloadOffset[cfr], targetRegister
> > +    end
> 
> Ditto.

Ditto from my end.  Instead of adding "if"'s everywhere, just abstract what you're really after: the CellOffset.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150324/00d4465f/attachment-0002.html>


More information about the webkit-unassigned mailing list