Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(204)

Issue 560903002: Improve x32 detection macro. (Closed)

Created:
6 years, 3 months ago by Nico
Modified:
6 years, 3 months ago
CC:
v8-dev, Toon Verwaest
Project:
v8
Visibility:
Public.

Description

Improve x32 detection macro. When targeting the Microsoft ABI in 64bit mode, clang defines __x86_64__ but doesn't define __LP64__ (Microsoft uses LLP64), so it would fall down the x32 path. cl.exe doesn't define __x86_64__ in the first place, so it didn't have this problem. Rather than trying to guess pointer size by looking at __x86_64__ and __LP64__, check for pointer size directly using __POINTER_SIZE__. This is defined by both gcc and clang, and eliminiates this problem. This should fix hundreds of "error(clang): unknown type name 'Atomic64'" when compiling v8 on Windows with clang for 64 bit. BUG=chromium:82385 LOG=n R=haitao.feng@intel.com, jochen@chromium.org Committed: https://code.google.com/p/v8/source/detail?r=23855

Patch Set 1 #

Total comments: 2

Patch Set 2 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -2 lines) Patch
M src/base/build_config.h View 1 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 10 (3 generated)
Nico
(see also http://llvm.org/PR20896)
6 years, 3 months ago (2014-09-10 17:56:26 UTC) #2
Nico
(from the llvm bug: https://sourceware.org/glibc/wiki/x32 recommends to check __ILP32__ instead, no idea if that's better ...
6 years, 3 months ago (2014-09-10 17:58:21 UTC) #3
hans
https://codereview.chromium.org/560903002/diff/1/src/base/build_config.h File src/base/build_config.h (right): https://codereview.chromium.org/560903002/diff/1/src/base/build_config.h#newcode27 src/base/build_config.h:27: #if defined(__x86_64__) && __SIZEOF_POINTER__ == 32 // Check for ...
6 years, 3 months ago (2014-09-10 18:05:44 UTC) #5
Nico
https://codereview.chromium.org/560903002/diff/1/src/base/build_config.h File src/base/build_config.h (right): https://codereview.chromium.org/560903002/diff/1/src/base/build_config.h#newcode27 src/base/build_config.h:27: #if defined(__x86_64__) && __SIZEOF_POINTER__ == 32 // Check for ...
6 years, 3 months ago (2014-09-10 18:08:20 UTC) #6
haitao.feng
LGTM
6 years, 3 months ago (2014-09-11 01:24:21 UTC) #7
jochen (gone - plz use gerrit)
lgtm
6 years, 3 months ago (2014-09-11 08:00:25 UTC) #9
jochen (gone - plz use gerrit)
6 years, 3 months ago (2014-09-11 08:01:32 UTC) #10
Message was sent while issue was closed.
Committed patchset #2 (id:20001) manually as 23855 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698