RESOLVED FIXED 116662
[CMake] Fix detection of x86_64 platform with MSVC 
https://bugs.webkit.org/show_bug.cgi?id=116662
Summary [CMake] Fix detection of x86_64 platform with MSVC
Patrick R. Gansterer
Reported 2013-05-23 05:59:09 PDT
[CMake] Fix detection of x86_64 platform with MSVC
Attachments
Patch (1.89 KB, patch)
2013-05-23 06:05 PDT, Patrick R. Gansterer 		gyuyoung.kim: review+
gyuyoung.kim: commit-queue-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Patrick R. Gansterer
Comment 1 2013-05-23 06:05:06 PDT
Created attachment 202671 [details] Patch
Laszlo Gombos
Comment 2 2013-06-20 10:28:05 PDT
Some ports are delegating build decisions to the C files and do not expose CPU variables to the build system. This logic is mostly already cross-platform. Can we explore that option for CMake as well ? Only CPU variable that seems to be used in CMake is WTF_CPU_ARM. Can the other WTF_CPU_XXX variables removed ? Can we remove WTF_CPU_ARM as well - for example assembler/ARMv7Assembler.cpp is already guarded with CPU(ARM_THUMB2), which seems to be a more specific guard.
Patrick R. Gansterer
Comment 3 2013-06-20 12:07:57 PDT
(In reply to comment #2) > Some ports are delegating build decisions to the C files and do not expose CPU variables to the build system. This logic is mostly already cross-platform. Can we explore that option for CMake as well ? > > Only CPU variable that seems to be used in CMake is WTF_CPU_ARM. Can the other WTF_CPU_XXX variables removed ? > > Can we remove WTF_CPU_ARM as well - for example assembler/ARMv7Assembler.cpp is already guarded with CPU(ARM_THUMB2), which seems to be a more specific guard. I need to add some (different) assembler files depending on the WTF_CPU to the build systems. AFAIK MS assembler has no possibility for a #if CPU(XXX), so I don't see an other chance. :-/
Patrick R. Gansterer
Comment 4 2013-07-30 05:38:55 PDT
ping?
Raphael Kubo da Costa (:rakuco)
Comment 5 2013-08-13 07:26:24 PDT
Comment on attachment 202671 [details] Patch LGTM, but I'd add a comment before the string(TOLOWER ...) with the same explanation you have in the ChangeLog.
Gyuyoung Kim
Comment 6 2013-08-14 23:07:58 PDT
Comment on attachment 202671 [details] Patch rs=me, but, please land after adjusting kubo's comment.
Patrick R. Gansterer
Comment 7 2013-09-04 02:39:22 PDT
Committed r154978: <http://trac.webkit.org/changeset/154978>
Note You need to log in before you can comment on or make changes to this bug.