Bug 135565

Summary: [JSC] Build fix for FTL on EFL after ftlopt merge
Product: WebKit Reporter: Dániel Bátyai <dbatyai.u-szeged>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cmarcelo, commit-queue, ggaren, llango.u-szeged, mark.lam, mhahnenberg
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
mark.lam: review-
Patch
none
Patch none

Dániel Bátyai
Reported 2014-08-04 07:16:38 PDT
Currently we don't have a good way of generating the required files for native call inlining.
Attachments
Patch (4.57 KB, patch)
2014-08-04 07:18 PDT, Dániel Bátyai 		no flags
DetailsFormatted DiffDiff
Patch (5.95 KB, patch)
2014-08-06 06:38 PDT, Dániel Bátyai 		mark.lam: review-
DetailsFormatted DiffDiff
Patch (5.93 KB, patch)
2014-08-06 07:55 PDT, Dániel Bátyai 		no flags
DetailsFormatted DiffDiff
Patch (5.93 KB, patch)
2014-08-06 07:57 PDT, Dániel Bátyai 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Dániel Bátyai
Comment 1 2014-08-04 07:18:04 PDT
Created attachment 235973 [details] Patch
Mark Lam
Comment 2 2014-08-05 10:39:03 PDT
Comment on attachment 235973 [details] Patch Can you please add a comment in the ChangeLog to indicate why this fix is needed? For example, say something about the feature of inlining native functions id dependent on features of the Clang compiler.
Mark Hahnenberg
Comment 3 2014-08-05 10:46:42 PDT
Maybe we should create a feature flag for the native inlining stuff? That might be a little clearer than just slapping COMPILER(CLANG) in a bunch of places.
Mark Lam
Comment 4 2014-08-05 10:48:27 PDT
(In reply to comment #3) > Maybe we should create a feature flag for the native inlining stuff? That might be a little clearer than just slapping COMPILER(CLANG) in a bunch of places. Yes, I agree. That is a better plan.
Dániel Bátyai
Comment 5 2014-08-06 06:38:30 PDT
Created attachment 236097 [details] Patch
Mark Lam
Comment 6 2014-08-06 07:15:59 PDT
Comment on attachment 236097 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=236097&action=review Also please rebase to ToT so that it’ll be easier to review this in context. Thanks. > Source/WTF/wtf/Platform.h:740 > +#if PLATFORM(EFL) > +#define ENABLE_FTL_NATIVE_CALL_INLINING 0 > +#else > +#define ENABLE_FTL_NATIVE_CALL_INLINING 1 > +#endif // PLATFORM(EFL) Please change this to be dependent on COMPILER(CLANG) instead of PLATFORM(EFL) since that is the true nature of the dependency. You could disable it for !COMPILER(CLANG) || PLATFORM(EFL) if you also don’t want to use it for EFL regardless of the compiler.
Dániel Bátyai
Comment 7 2014-08-06 07:55:53 PDT
Created attachment 236103 [details] Patch
Dániel Bátyai
Comment 8 2014-08-06 07:57:40 PDT
Created attachment 236104 [details] Patch Fixed a typo
Mark Lam
Comment 9 2014-08-06 08:02:50 PDT
Comment on attachment 236104 [details] Patch r=me
WebKit Commit Bot
Comment 10 2014-08-06 08:45:19 PDT
Comment on attachment 236104 [details] Patch Clearing flags on attachment: 236104 Committed r172149: <http://trac.webkit.org/changeset/172149>
WebKit Commit Bot
Comment 11 2014-08-06 08:45:23 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.