ARM: Makefile/gyp update allowing better control of ARM specific options. (issue 14263018)

6 views
Skip to first unread message

rodolph....@gmail.com

unread,
Apr 15, 2013, 2:58:51 PM4/15/13
to da...@chromium.org, ul...@chromium.org, v8-...@googlegroups.com
Reviewers: danno, ulan,

Description:
ARM: Makefile/gyp update allowing better control of ARM specific options.

This patch defines new makefile command line paramaters to better control
the
ARM specific options. The new paramters are
* armfpu = vfp, vfpv3-d16, vfpv3, neon.
* armfloatabi = softfp, hard
* armneon = on
* armthumb = on, off
* armtest = on
One existing paratemer has been modified:
* armv7 = true, false
A number of parameters have been deprecated (but are still working):
* hardfp = on, off
* vfp2 = off
* vfp3 = off

the armtest paratmer when set to "on" will lock the options used during
compile
time at runtime. This allows for example to easily test the ARMv6 build on
an
ARMv7 platform without having to worry about features detected at runtime.
When
not specified the compiler default will be used meaning it is not necessary
anymore to specify hardfp=on when natively building on an hardfp platform.

The shell help now prints the target options and features detected.

BUG=none
TEST=none


Please review this at https://codereview.chromium.org/14263018/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files:
M Makefile
M build/common.gypi
M src/arm/assembler-arm.h
M src/arm/assembler-arm.cc
M src/arm/constants-arm.h
M src/arm/macro-assembler-arm.h
M src/flag-definitions.h
M src/flags.cc
M src/platform-linux.cc
M tools/gyp/v8.gyp


ul...@chromium.org

unread,
Apr 16, 2013, 6:12:18 AM4/16/13
to rodolph....@gmail.com, da...@chromium.org, jkum...@chromium.org, v8-...@googlegroups.com
I like this change! Adding Jakob in case I missed something in gyp files.


https://codereview.chromium.org/14263018/diff/1/src/arm/assembler-arm.cc
File src/arm/assembler-arm.cc (right):

https://codereview.chromium.org/14263018/diff/1/src/arm/assembler-arm.cc#newcode115
src/arm/assembler-arm.cc:115: PrintFeatures();
Debug print?

https://codereview.chromium.org/14263018/diff/1/src/arm/assembler-arm.cc#newcode213
src/arm/assembler-arm.cc:213: # if defined CAN_USE_ARMV7_INSTRUCTIONS
Shouldn't it be CAN_USE_VFP3_INSTRUCTIONS?

https://codereview.chromium.org/14263018/diff/1/src/arm/constants-arm.h
File src/arm/constants-arm.h (right):

https://codereview.chromium.org/14263018/diff/1/src/arm/constants-arm.h#newcode47
src/arm/constants-arm.h:47: # define CAN_USE_VFP3_INSTRUCTIONS
Does __ARM_ARCH_7__ imply VFP3?

https://codereview.chromium.org/14263018/

rodolph....@gmail.com

unread,
Apr 16, 2013, 6:49:35 AM4/16/13
to da...@chromium.org, ul...@chromium.org, jkum...@chromium.org, v8-...@googlegroups.com
Let me know what you think about debug print and snapshots (see my reply to
your
comment) and I'll upload a new patch.
On 2013/04/16 10:12:18, ulan wrote:
> Debug print?

I find it useful to see these info printed when I do a release build
too. I am fine with printing it only in debug mode though, let me know
what you prefer.

https://codereview.chromium.org/14263018/diff/1/src/arm/assembler-arm.cc#newcode213
src/arm/assembler-arm.cc:213: # if defined CAN_USE_ARMV7_INSTRUCTIONS
On 2013/04/16 10:12:18, ulan wrote:
> Shouldn't it be CAN_USE_VFP3_INSTRUCTIONS?

Done.

https://codereview.chromium.org/14263018/diff/1/src/arm/constants-arm.h
File src/arm/constants-arm.h (right):

https://codereview.chromium.org/14263018/diff/1/src/arm/constants-arm.h#newcode47
src/arm/constants-arm.h:47: # define CAN_USE_VFP3_INSTRUCTIONS
On 2013/04/16 10:12:18, ulan wrote:
> Does __ARM_ARCH_7__ imply VFP3?

Here yes, V8 requires VFP support and VFP + ARMv7 imply VFPv3

https://codereview.chromium.org/14263018/

jkum...@chromium.org

unread,
Apr 16, 2013, 7:36:08 AM4/16/13
to rodolph....@gmail.com, da...@chromium.org, ul...@chromium.org, v8-...@googlegroups.com
Build system stuff LGTM, with a suggestion.


https://codereview.chromium.org/14263018/diff/1/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/14263018/diff/1/build/common.gypi#newcode155
build/common.gypi:155: 'target_conditions': [
Refactoring suggestion (feel free to ignore if you think it wouldn't be
better, or if it's not possible for some reason I'm overlooking):
It seems all these conditional settings (up to and excluding ARM_TEST)
apply to the target toolset only. You could pull up the
_toolset=="target" target_condition to wrap everything, that would avoid
spelling it out over and over again.

https://codereview.chromium.org/14263018/

rodolph....@gmail.com

unread,
Apr 16, 2013, 10:15:56 AM4/16/13
to da...@chromium.org, ul...@chromium.org, jkum...@chromium.org, v8-...@googlegroups.com
Ulan, let me know how you feel about printing features when generating
snapshots.
On 2013/04/16 11:36:08, Jakob wrote:
> Refactoring suggestion (feel free to ignore if you think it wouldn't
be better,
> or if it's not possible for some reason I'm overlooking):
> It seems all these conditional settings (up to and excluding ARM_TEST)
apply to
> the target toolset only. You could pull up the _toolset=="target"
> target_condition to wrap everything, that would avoid spelling it out
over and
> over again.

Done.

https://codereview.chromium.org/14263018/

ul...@chromium.org

unread,
Apr 16, 2013, 10:20:50 AM4/16/13
to rodolph....@gmail.com, da...@chromium.org, jkum...@chromium.org, v8-...@googlegroups.com
LGTM
On 2013/04/16 10:49:35, Rodolph Perfetta wrote:
> On 2013/04/16 10:12:18, ulan wrote:
> > Debug print?

> I find it useful to see these info printed when I do a release build
too. I am
> fine with printing it only in debug mode though, let me know what you
prefer.


I see. I don't mind printing it.

https://codereview.chromium.org/14263018/
Reply all
Reply to author
Forward
0 new messages