On 2012/07/25 09:48:49, Yang wrote:
On 2012/07/25 07:49:06, Sven Panne wrote:
> http://codereview.chromium.org/10818026/diff/1/src/arm/assembler-arm.cc
> File src/arm/assembler-arm.cc (right):
>
>
http://codereview.chromium.org/10818026/diff/1/src/arm/assembler-arm.cc#newcode50
> src/arm/assembler-arm.cc:50: unsigned CpuFeatures::supported_ = 0;
> We should either use uint64_t here or use unsigned below (same for other
> uint64_t/unsigned mixing below).
>
> http://codereview.chromium.org/10818026/diff/1/src/arm/assembler-arm.h
> File src/arm/assembler-arm.h (right):
>
>
http://codereview.chromium.org/10818026/diff/1/src/arm/assembler-arm.h#newcode513
> src/arm/assembler-arm.h:513: if (f == VFP2 && !FLAG_enable_vfp3) return
false;
> This is confusing, I think we should add a new flag for VFP2 or rename
the
> existing one. In any case our build bots have to be changed accordingly.
>
>
http://codereview.chromium.org/10818026/diff/1/src/arm/assembler-arm.h#newcode539
> src/arm/assembler-arm.h:539: // VFP2 and ARMv2 are implied by VFP3.
> ARMv7?
>
>
http://codereview.chromium.org/10818026/diff/1/src/arm/macro-assembler-arm.cc
> File src/arm/macro-assembler-arm.cc (right):
>
>
http://codereview.chromium.org/10818026/diff/1/src/arm/macro-assembler-arm.cc#newcode790
> src/arm/macro-assembler-arm.cc:790: } else if (false &&
> CpuFeatures::IsSupported(VFP3)) {
> As discussed offline, this duplicates some fallback code in
Assembler:vmov,
so
> we should handle VFP2-only platforms there.
I addressed Sven's comments. Following Rodolph's comments (crankshaft has
been
consciously ruled out for processors without VFP3), I decided to scale
back
this
CL to only lower the requirements to VFP2 where possible.
Please take another quick look.
http://codereview.chromium.org/10818026/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
