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

Issue 12635: * Fixed exclusion of IA32-specific tests on ARM architecture. (Closed)

Created:
12 years ago by Lasse Reichstein
Modified:
9 years, 6 months ago
Reviewers:
iposva, Erik Corry
CC:
v8-dev
Visibility:
Public.

Description

IA32-tests won't be compiled on ARM (and not just not run).

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+8 lines, -15 lines) Patch
M src/regexp-macro-assembler-ia32.h View 1 chunk +1 line, -1 line 1 comment Download
M src/regexp-macro-assembler-ia32.cc View 1 chunk +0 lines, -1 line 0 comments Download
M test/cctest/cctest.h View 1 chunk +0 lines, -9 lines 0 comments Download
M test/cctest/test-regexp.cc View 5 chunks +7 lines, -4 lines 1 comment Download

Messages

Total messages: 2 (0 generated)
Lasse Reichstein
Review please.
12 years ago (2008-11-25 13:50:18 UTC) #1
iposva
12 years ago (2008-11-25 15:44:10 UTC) #2
Please fix the problems with this change. I have not been watching the branch
while you guys were developing, but now that it is in bleeding_edge you need to
keep this clean.

Thanks,
-Ivan

http://codereview.chromium.org/12635/diff/1/3
File src/regexp-macro-assembler-ia32.h (right):

http://codereview.chromium.org/12635/diff/1/3#newcode31
Line 31: #if !(defined(ARM) || defined(__arm__) || defined(__thumb__))
This is just plain wrong. There should never be any reason to check for ARM in a
ia32 specific file. This indicates that there is a problem with your
abstractions or include files.

http://codereview.chromium.org/12635/diff/1/5
File test/cctest/test-regexp.cc (right):

http://codereview.chromium.org/12635/diff/1/5#newcode723
Line 723: #if !(defined(ARM) || defined(__arm__) || defined(__thumb__))
It should be sufficient to only test for "defined(ARM)" in general. __arm__ and
__thumb__ should only be tested if the code has to be different for the actual
ARM cpu running (Simulator, Thumb or general ARM).

Powered by Google App Engine
This is Rietveld 408576698