Bug 144304 - Fully replace ENABLE_LLINT_C_LOOP with ENABLE_JIT
Summary: Fully replace ENABLE_LLINT_C_LOOP with ENABLE_JIT
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Enhancement
Assignee: Michael Catanzaro
URL:
Keywords:
Depends on: 144359
Blocks:
  Show dependency treegraph
 
Reported: 2015-04-27 18:32 PDT by Michael Catanzaro
Modified: 2015-06-01 11:04 PDT (History)
11 users (show)

See Also:


Attachments
Patch (37.51 KB, patch)
2015-04-27 18:50 PDT, Michael Catanzaro
no flags Details | Formatted Diff | Diff
Patch (37.02 KB, patch)
2015-04-27 20:27 PDT, Michael Catanzaro
no flags Details | Formatted Diff | Diff
Patch (36.67 KB, patch)
2015-04-27 20:28 PDT, Michael Catanzaro
no flags Details | Formatted Diff | Diff
Patch (35.68 KB, patch)
2015-04-28 11:19 PDT, Michael Catanzaro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2015-04-27 18:32:03 PDT
In bug #143998 we removed ENABLE_LLINT_C_LOOP from the CMake build; now let's remove it completely.
Comment 1 Michael Catanzaro 2015-04-27 18:50:35 PDT
Created attachment 251805 [details]
Patch
Comment 2 Michael Catanzaro 2015-04-27 20:27:33 PDT
Created attachment 251813 [details]
Patch
Comment 3 Michael Catanzaro 2015-04-27 20:28:42 PDT
Created attachment 251814 [details]
Patch
Comment 4 Csaba Osztrogonác 2015-04-28 02:25:24 PDT
There are CLOOP buildbots on https://build.webkit.org/waterfall which still 
use --cloop option, they should be changed to --no-jit. But in this case 
the buildmaster should be restarted immediately after the patch landed.

- https://trac.webkit.org/browser/trunk/Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg#L233
- https://trac.webkit.org/browser/trunk/Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg#L491

Maybe it would be less intrusive if we keep --cloop 
option which could set ENABLE_JIT to 0.
Comment 5 Michael Catanzaro 2015-04-28 11:18:07 PDT
OK, let's keep the --cloop argument to build-jsc then.

I was hoping EWS would tell us if there is a problem with this patch, but I guess I'll need to keep an eye on that waterfall after landing.
Comment 6 Michael Catanzaro 2015-04-28 11:19:49 PDT
Created attachment 251867 [details]
Patch
Comment 7 Geoffrey Garen 2015-04-28 11:25:10 PDT
Sounds like a good idea. Let's see if it builds.
Comment 8 Michael Catanzaro 2015-04-28 12:56:25 PDT
It's fine by EWS at least. If I get r+ then I will check the waterfall after this lands and roll it out if the two cloop bots don't like it, but I bet it'll be fine.
Comment 9 Geoffrey Garen 2015-04-28 14:42:14 PDT
Comment on attachment 251867 [details]
Patch

r=me
Comment 10 Michael Catanzaro 2015-04-28 15:51:43 PDT
Comment on attachment 251867 [details]
Patch

Clearing flags on attachment: 251867

Committed r183514: <http://trac.webkit.org/changeset/183514>
Comment 11 Michael Catanzaro 2015-04-28 15:51:50 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Michael Catanzaro 2015-04-28 16:00:04 PDT
It broke the cloop bots:

"Cannot build FTL JIT without baseline JIT support!"

I will fix it. Ossy, thanks for mentioning that these exist, otherwise I would have had no clue.
Comment 13 WebKit Commit Bot 2015-04-28 16:02:45 PDT
Re-opened since this is blocked by bug 144359
Comment 14 Michael Catanzaro 2015-04-28 16:17:39 PDT
Committed r183516: <http://trac.webkit.org/changeset/183516>
Comment 15 Mark Lam 2015-06-01 10:00:43 PDT
(In reply to comment #14)
> Committed r183516: <http://trac.webkit.org/changeset/183516>

This broke the --cloop feature.  The CLoop bot has not been building the CLoop ever since.  Will fix in https://bugs.webkit.org/show_bug.cgi?id=145516.
Comment 16 Michael Catanzaro 2015-06-01 11:04:07 PDT
Oops, I was checking the cloop bots, but I guess "cloop bots are green and happy" is different from "cloop bots are actually testing cloop." :( Thanks for fixing.