Bug 126790 - CStack: Fix 64-bit C Loop LLINT
Summary: CStack: Fix 64-bit C Loop LLINT
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords:
: 126007 126191 126392 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-01-10 16:24 PST by Mark Lam
Modified: 2014-01-13 20:49 PST (History)
5 users (show)

See Also:


Attachments
the patch. (42.90 KB, patch)
2014-01-10 17:56 PST, Mark Lam
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2014-01-10 16:24:05 PST
Patch coming soon.
Comment 1 Mark Lam 2014-01-10 17:56:33 PST
Created attachment 220913 [details]
the patch.
Comment 2 Geoffrey Garen 2014-01-13 16:39:08 PST
Comment on attachment 220913 [details]
the patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=220913&action=review

r=me

> Source/JavaScriptCore/llint/LLIntOpcode.h:48
> +    macro(llint_synthesized_label_1, 1) \
> +    macro(llint_synthesized_label_2, 1) \
> +    macro(llint_synthesized_label_3, 1) \
> +    macro(llint_synthesized_label_4, 1) \
> +    macro(llint_synthesized_label_5, 1) \
> +    macro(llint_synthesized_label_6, 1) \
> +    macro(llint_synthesized_label_7, 1) \

Let's call these "llint_cloop_did_return_from_js_N".

> Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:93
> +#if 0

Let's give this a name, like ENABLE_OPCODE_TRACING.

> Source/JavaScriptCore/offlineasm/asm.rb:48
> +        @numSynthesizedGlobalLabels = 0

Let's call this "didReturnFromJSLabelCounter", and move into cloop.rb's Instruction class, using @@ syntax for a per-class variable.

> Source/JavaScriptCore/offlineasm/asm.rb:228
> +    def numSynthesizedGlobalLabels()
> +        @numSynthesizedGlobalLabels
> +    end
> +    def incNumSynthesizedGlobalLabels()
> +        @numSynthesizedGlobalLabels += 1
> +    end

See above renames.
Comment 3 Mark Lam 2014-01-13 17:05:08 PST
Thanks for the review.  The feedback has been applied.  Landed in r161927 on the jsCStack branch: <http://trac.webkit.org/r161927>.
Comment 4 Mark Lam 2014-01-13 17:06:41 PST
*** Bug 126392 has been marked as a duplicate of this bug. ***
Comment 5 Mark Lam 2014-01-13 17:10:07 PST
*** Bug 126007 has been marked as a duplicate of this bug. ***
Comment 6 Mark Lam 2014-01-13 17:26:36 PST
*** Bug 126191 has been marked as a duplicate of this bug. ***
Comment 7 Mark Lam 2014-01-13 20:49:53 PST
(In reply to comment #3)
> Thanks for the review.  The feedback has been applied.  Landed in r161927 on the jsCStack branch: <http://trac.webkit.org/r161927>.

The merge had a conflict that went uncaught by svn. This resulted in a redundant push of lr and a redundant pop of cfr, and this breaks the C loop runs (as well as ARMv7 and other similar architectures).  This is now fixed in r161942: <http://trac.webkit.org/r161942>.