Bug 144304

Summary: Fully replace ENABLE_LLINT_C_LOOP with ENABLE_JIT
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: JavaScriptCoreAssignee: Michael Catanzaro <mcatanzaro>
Status: RESOLVED FIXED    
Severity: Enhancement CC: benjamin, cmarcelo, commit-queue, dbates, fpizlo, ggaren, mark.lam, mcatanzaro, mrobinson, ossy, tpopela
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on: 144359    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Michael Catanzaro
Reported 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.
Attachments
Patch (37.51 KB, patch)
2015-04-27 18:50 PDT, Michael Catanzaro 		no flags
DetailsFormatted DiffDiff
Patch (37.02 KB, patch)
2015-04-27 20:27 PDT, Michael Catanzaro 		no flags
DetailsFormatted DiffDiff
Patch (36.67 KB, patch)
2015-04-27 20:28 PDT, Michael Catanzaro 		no flags
DetailsFormatted DiffDiff
Patch (35.68 KB, patch)
2015-04-28 11:19 PDT, Michael Catanzaro 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Michael Catanzaro
Comment 1 2015-04-27 18:50:35 PDT
Created attachment 251805 [details] Patch
Michael Catanzaro
Comment 2 2015-04-27 20:27:33 PDT
Created attachment 251813 [details] Patch
Michael Catanzaro
Comment 3 2015-04-27 20:28:42 PDT
Created attachment 251814 [details] Patch
Csaba Osztrogonác
Comment 4 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.
Michael Catanzaro
Comment 5 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.
Michael Catanzaro
Comment 6 2015-04-28 11:19:49 PDT
Created attachment 251867 [details] Patch
Geoffrey Garen
Comment 7 2015-04-28 11:25:10 PDT
Sounds like a good idea. Let's see if it builds.
Michael Catanzaro
Comment 8 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.
Geoffrey Garen
Comment 9 2015-04-28 14:42:14 PDT
Comment on attachment 251867 [details] Patch r=me
Michael Catanzaro
Comment 10 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>
Michael Catanzaro
Comment 11 2015-04-28 15:51:50 PDT
All reviewed patches have been landed. Closing bug.
Michael Catanzaro
Comment 12 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.
WebKit Commit Bot
Comment 13 2015-04-28 16:02:45 PDT
Re-opened since this is blocked by bug 144359
Michael Catanzaro
Comment 14 2015-04-28 16:17:39 PDT
Committed r183516: <http://trac.webkit.org/changeset/183516>
Mark Lam
Comment 15 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.
Michael Catanzaro
Comment 16 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.
Note You need to log in before you can comment on or make changes to this bug.