RESOLVED FIXED158661
[JSC] Move calling convention flags to WTF
https://bugs.webkit.org/show_bug.cgi?id=158661
Summary [JSC] Move calling convention flags to WTF
Yusuke Suzuki
Reported 2016-06-11 07:49:49 PDT
[JSC] Move calling convension flags to WTF
Attachments
Patch (5.29 KB, patch)
2016-06-11 07:54 PDT, Yusuke Suzuki
keith_miller: review+
Patch for landing (5.56 KB, patch)
2016-06-11 08:46 PDT, Yusuke Suzuki
no flags
Patch for landing (5.82 KB, patch)
2016-06-13 08:28 PDT, Yusuke Suzuki
no flags
Windows EWS checking (5.77 KB, patch)
2016-06-13 09:34 PDT, Yusuke Suzuki
no flags
Windows EWS checking (5.82 KB, patch)
2016-06-13 10:01 PDT, Yusuke Suzuki
no flags
Windows EWS checking (6.28 KB, patch)
2016-06-15 07:33 PDT, Yusuke Suzuki
no flags
Windows EWS checking (37.53 KB, patch)
2016-06-15 08:11 PDT, Yusuke Suzuki
no flags
Windows EWS checking (37.65 KB, patch)
2016-06-15 08:33 PDT, Yusuke Suzuki
no flags
Yusuke Suzuki
Comment 1 2016-06-11 07:54:50 PDT
Keith Miller
Comment 2 2016-06-11 08:25:09 PDT
Comment on attachment 281100 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=281100&action=review r=me with comments. > Source/JavaScriptCore/ChangeLog:3 > + [JSC] Move calling convension flags to WTF typo: convension => convention > Source/JavaScriptCore/ChangeLog:8 > + Due to some calling convension flags and JIT_OPERATION falgs, MathCommon.h inclues MacroAssemblerCodeRef and JITOperations.h. ditto. > Source/JavaScriptCore/ChangeLog:10 > + As the same to other calling convension flags like JSC_HOST_CALL, we move these flags to WTF. I think this should be: As with other calling convention flags like JSC_HOST_CALL, these flags should be in WTF
Yusuke Suzuki
Comment 3 2016-06-11 08:42:02 PDT
Comment on attachment 281100 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=281100&action=review Thanks!! >> Source/JavaScriptCore/ChangeLog:3 >> + [JSC] Move calling convension flags to WTF > > typo: convension => convention Oooops, fixed. >> Source/JavaScriptCore/ChangeLog:8 >> + Due to some calling convension flags and JIT_OPERATION falgs, MathCommon.h inclues MacroAssemblerCodeRef and JITOperations.h. > > ditto. fixed. >> Source/JavaScriptCore/ChangeLog:10 >> + As the same to other calling convension flags like JSC_HOST_CALL, we move these flags to WTF. > > I think this should be: > > As with other calling convention flags like JSC_HOST_CALL, these flags should be in WTF Looks very nice! Changed.
Yusuke Suzuki
Comment 4 2016-06-11 08:46:12 PDT
Created attachment 281101 [details] Patch for landing Check Windows port EWS
Mark Lam
Comment 5 2016-06-11 10:47:58 PDT
Comment on attachment 281100 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=281100&action=review >>> Source/JavaScriptCore/ChangeLog:8 >>> + Due to some calling convension flags and JIT_OPERATION falgs, MathCommon.h inclues MacroAssemblerCodeRef and JITOperations.h. >> >> ditto. > > fixed. Another typo: /falgs/flags/.
Yusuke Suzuki
Comment 6 2016-06-13 08:28:20 PDT
Created attachment 281174 [details] Patch for landing Check Windows port EWS
Yusuke Suzuki
Comment 7 2016-06-13 09:34:51 PDT
Created attachment 281177 [details] Windows EWS checking Check Windows port EWS, part 2...
Yusuke Suzuki
Comment 8 2016-06-13 10:01:21 PDT
Created attachment 281179 [details] Windows EWS checking Check Windows port EWS, part 3...
Yusuke Suzuki
Comment 9 2016-06-15 07:33:57 PDT
Created attachment 281360 [details] Windows EWS checking Check Windows port EWS, part 4...
Yusuke Suzuki
Comment 10 2016-06-15 08:11:54 PDT
Created attachment 281361 [details] Windows EWS checking Check Windows port EWS, part 5...
Yusuke Suzuki
Comment 11 2016-06-15 08:14:27 PDT
Ah OK, it seems that I've just found the existing bug in Windows port. In Windows port, previously, we typedef the JIT operations as follows, typedef EncodeJSValue JIT_OPERATION (*J_JITOperation_E)(ExecState*); And we assumed that JIT_OPERATION is CDECL (And CDECL is __cdecl in Windows port). But it was not correct, since typedef EncodeJSValue __cdecl (*J_JITOperation_E(ExecState*); is syntax incorrect. In previous Windows port, we accidentally define JIT_OPERATION here as "". (Maybe, CALLING_CONVENTION_IS_STDCALL is not defined accidentally in JITOperation.h) So, the above typedef is recognized as typedef EncodeJSValue (*J_JITOperation_E)(ExecState*); I checked it by manually typedefining `typedef EncodeJSValue __cdecl (*J_JITOperation_E)(ExecState*);` and seeing the build failure on Windows EWS. To correct this, we need to define it as follows in Windows MSVC environment. typedef EncodeJSValue (__cdecl *J_JITOperation_E)(ExecState*); So, we need to typedef the function pointer as follows, typedef EncodeJSValue (JIT_OPERATION *J_JITOperation_E)(ExecState*);
Yusuke Suzuki
Comment 12 2016-06-15 08:33:56 PDT
Created attachment 281362 [details] Windows EWS checking Check Windows port EWS, part 6... Maybe, the last check.
Yusuke Suzuki
Comment 13 2016-06-15 09:27:16 PDT
Comment on attachment 281100 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=281100&action=review >>>> Source/JavaScriptCore/ChangeLog:8 >>>> + Due to some calling convension flags and JIT_OPERATION falgs, MathCommon.h inclues MacroAssemblerCodeRef and JITOperations.h. >>> >>> ditto. >> >> fixed. > > Another typo: /falgs/flags/. Thanks! Fixed.
Yusuke Suzuki
Comment 14 2016-06-15 09:28:26 PDT
Yusuke Suzuki
Comment 15 2016-06-15 09:43:58 PDT
Note You need to log in before you can comment on or make changes to this bug.