Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1589933002:
Fixed compilation on mingw. (Closed)
|
Created:
4 years, 11 months ago by cjacek Modified:
4 years, 9 months ago Reviewers:
bungeman-skia CC:
reviews_skia.org Base URL:
https://skia.googlesource.com/skia.git@master Target Ref:
refs/heads/master Project:
skia Visibility:
Public. |
DescriptionFixed compilation on mingw.
Found and tested in mingw Firefox build. There are two problems:
- GCC doesn't support __vectorcall calling convention, so its usage needs to be #ifdefed
- GetProcAddress returns FARPROC type (a function pointer) and GCC requires an explicit cast to void*
Committed: https://skia.googlesource.com/skia/+/c721196067b78226b97709998a96e0f037d8fb64
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1589933002
Patch Set 1 #
Total comments: 2
Patch Set 2 : Fixed compilation on mingw. #Patch Set 3 : Minor mingw cross compilation fix. #Messages
Total messages: 18 (9 generated)
Description was changed from ========== Fixed compilation on mingw. ========== to ========== Fixed compilation on mingw. GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== Fixed compilation on mingw. GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Fixed compilation on mingw. GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Found and tested in mingw Firefox build. There are two problems: - GCC doesn't support __vectorcall calling convention, so its usage needs to be #ifdefed - GetProcAddress returns FARPROC type (a function pointer) and GCC requires an explicit cast to void* ==========
cjacek@gmail.com changed reviewers: + bungeman@google.com
https://codereview.chromium.org/1589933002/diff/1/include/private/SkTLogic.h File include/private/SkTLogic.h (right): https://codereview.chromium.org/1589933002/diff/1/include/private/SkTLogic.h#... include/private/SkTLogic.h:61: #endif I would really like to avoid another nesting ifdef here, it's getting hard to read. This was originally structured on the idea that it would be clearer to have each individual platform (or set of calling conventions) in one block. As it turns out, this is a bit of a mess. I think it's clearer to restructure this, replacing the above lines 48 to 60 with template <typename R, typename... Args> struct is_function<R __cdecl (Args...)> : std::true_type {}; # if defined(_M_IX86) template <typename R, typename... Args> struct is_function<R __stdcall (Args...)> : std::true_type {}; template <typename R, typename... Args> struct is_function<R __fastcall (Args...)> : std::true_type {}; # endif # if defined(_MSC_VER) && (SK_CPU_SSE_LEVEL >= SK_CPU_SSE_LEVEL_SSE2) template <typename R, typename... Args> struct is_function<R __vectorcall (Args...)> : std::true_type {}; # endif https://codereview.chromium.org/1589933002/diff/1/src/ports/SkOSLibrary_win.cpp File src/ports/SkOSLibrary_win.cpp (right): https://codereview.chromium.org/1589933002/diff/1/src/ports/SkOSLibrary_win.c... src/ports/SkOSLibrary_win.cpp:19: return (void*)::GetProcAddress((HMODULE)library, functionName); Can we use a c++ cast here? Something like reinterpret_cast<void*>(GetProcAddress(...)) or *reinterpret_cast<void**>(GetProcAddress(...)).
Thanks for the review and sorry for the delay. I attached a fixed version.
The CQ bit was checked by bungeman@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1589933002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1589933002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Fixed compilation on mingw. GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Found and tested in mingw Firefox build. There are two problems: - GCC doesn't support __vectorcall calling convention, so its usage needs to be #ifdefed - GetProcAddress returns FARPROC type (a function pointer) and GCC requires an explicit cast to void* ========== to ========== Fixed compilation on mingw. Found and tested in mingw Firefox build. There are two problems: - GCC doesn't support __vectorcall calling convention, so its usage needs to be #ifdefed - GetProcAddress returns FARPROC type (a function pointer) and GCC requires an explicit cast to void* ==========
The CQ bit was checked by bungeman@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1589933002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1589933002/20001
Note for Reviewers: The CQ is waiting for an approval. If you believe that the CL is not ready yet, or if you would like to L-G-T-M with comments then please uncheck the CQ checkbox. Waiting for LGTM from valid reviewer(s) till 2016-01-25 21:23 UTC
lgtm
Message was sent while issue was closed.
Description was changed from ========== Fixed compilation on mingw. Found and tested in mingw Firefox build. There are two problems: - GCC doesn't support __vectorcall calling convention, so its usage needs to be #ifdefed - GetProcAddress returns FARPROC type (a function pointer) and GCC requires an explicit cast to void* ========== to ========== Fixed compilation on mingw. Found and tested in mingw Firefox build. There are two problems: - GCC doesn't support __vectorcall calling convention, so its usage needs to be #ifdefed - GetProcAddress returns FARPROC type (a function pointer) and GCC requires an explicit cast to void* Committed: https://skia.googlesource.com/skia/+/c721196067b78226b97709998a96e0f037d8fb64 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://skia.googlesource.com/skia/+/c721196067b78226b97709998a96e0f037d8fb64
Message was sent while issue was closed.
Description was changed from ========== Fixed compilation on mingw. Found and tested in mingw Firefox build. There are two problems: - GCC doesn't support __vectorcall calling convention, so its usage needs to be #ifdefed - GetProcAddress returns FARPROC type (a function pointer) and GCC requires an explicit cast to void* Committed: https://skia.googlesource.com/skia/+/c721196067b78226b97709998a96e0f037d8fb64 ========== to ========== Fixed compilation on mingw. Found and tested in mingw Firefox build. There are two problems: - GCC doesn't support __vectorcall calling convention, so its usage needs to be #ifdefed - GetProcAddress returns FARPROC type (a function pointer) and GCC requires an explicit cast to void* Committed: https://skia.googlesource.com/skia/+/c721196067b78226b97709998a96e0f037d8fb64 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== |
