From 46000a12449f09042be60e87dfc76044aec3ad92 Mon Sep 17 00:00:00 2001 From: Mostyn Bramley-Moore Date: Wed, 13 Sep 2017 23:16:25 +0200 Subject: [PATCH] [jumbo] fix arm64 builds Previously instructions-arm64.h was alternatively defining or declaring some constants based on whether or not ARM64_DEFINE_FP_STATICS was defined, and it was assumed that exactly one file would include this header with the macro defined. In jumbo builds, the header guards in instructions-arm64.h meant that the resulting state of the header file would be whichever of the two cases that appeared first in the compilation unit. This would cause multiple definitions in some cases and no definitions in some other cases (or if you were really lucky, it would work out ok). Let's move these constants to a separate source file temporarily, to be excluded from jumbo compilation units. This code should eventually be replaced with a cleaner solution. Bug: chromium:746958 Change-Id: I7edb1821ef408afd50c6b236d63d3c07f955b58f Reviewed-on: https://chromium-review.googlesource.com/663898 Commit-Queue: Mostyn Bramley-Moore Reviewed-by: Jakob Kummerow Cr-Commit-Position: refs/heads/master@{#48003} --- BUILD.gn | 8 ++++ src/arm64/instructions-arm64-constants.cc | 46 +++++++++++++++++++++++ src/arm64/instructions-arm64.cc | 3 -- src/arm64/instructions-arm64.h | 44 +++++++--------------- src/v8.gyp | 1 + 5 files changed, 68 insertions(+), 34 deletions(-) create mode 100644 src/arm64/instructions-arm64-constants.cc diff --git a/BUILD.gn b/BUILD.gn index b70ef29336..63382ba536 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -2224,6 +2224,7 @@ v8_source_set("v8_base") { "src/arm64/eh-frame-arm64.cc", "src/arm64/frame-constants-arm64.cc", "src/arm64/frame-constants-arm64.h", + "src/arm64/instructions-arm64-constants.cc", "src/arm64/instructions-arm64.cc", "src/arm64/instructions-arm64.h", "src/arm64/instrument-arm64.cc", @@ -2250,6 +2251,13 @@ v8_source_set("v8_base") { "src/regexp/arm64/regexp-macro-assembler-arm64.cc", "src/regexp/arm64/regexp-macro-assembler-arm64.h", ] + if (use_jumbo_build) { + jumbo_excluded_sources += [ + # TODO(mostynb@opera.com): fix this code so it doesn't need + # to be excluded, see the comments inside. + "src/arm64/instructions-arm64-constants.cc", + ] + } } else if (v8_current_cpu == "mips" || v8_current_cpu == "mipsel") { sources += [ ### gcmole(arch:mipsel) ### "src/compiler/mips/code-generator-mips.cc", diff --git a/src/arm64/instructions-arm64-constants.cc b/src/arm64/instructions-arm64-constants.cc new file mode 100644 index 0000000000..5f1b49fbdc --- /dev/null +++ b/src/arm64/instructions-arm64-constants.cc @@ -0,0 +1,46 @@ +// Copyright 2017 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include + +namespace v8 { +namespace internal { + +// ISA constants. -------------------------------------------------------------- + +// The following code initializes float/double variables with bit patterns +// without using static initializers (which is surprisingly difficult in +// C++). These variables are used by client code as extern float16, +// extern float and extern double types, which works because (I think) the +// linker ignores the types. This is kept in a separate source file to +// avoid breaking jumbo builds. +// +// TODO(mostynb): replace these with std::numeric_limits constexpr's where +// possible, and figure out how to replace *DefaultNaN with something clean, +// then move this code back into instructions-arm64.cc with the same types +// that client code uses. + +extern const uint16_t kFP16PositiveInfinity = 0x7c00; +extern const uint16_t kFP16NegativeInfinity = 0xfc00; +extern const uint32_t kFP32PositiveInfinity = 0x7f800000; +extern const uint32_t kFP32NegativeInfinity = 0xff800000; +extern const uint64_t kFP64PositiveInfinity = 0x7ff0000000000000UL; +extern const uint64_t kFP64NegativeInfinity = 0xfff0000000000000UL; + +// This value is a signalling NaN as both a double and as a float (taking the +// least-significant word). +extern const uint64_t kFP64SignallingNaN = 0x7ff000007f800001; +extern const uint32_t kFP32SignallingNaN = 0x7f800001; + +// A similar value, but as a quiet NaN. +extern const uint64_t kFP64QuietNaN = 0x7ff800007fc00001; +extern const uint32_t kFP32QuietNaN = 0x7fc00001; + +// The default NaN values (for FPCR.DN=1). +extern const uint64_t kFP64DefaultNaN = 0x7ff8000000000000UL; +extern const uint32_t kFP32DefaultNaN = 0x7fc00000; +extern const uint16_t kFP16DefaultNaN = 0x7e00; + +} // namespace internal +} // namespace v8 diff --git a/src/arm64/instructions-arm64.cc b/src/arm64/instructions-arm64.cc index f4dbd75533..d6f106b800 100644 --- a/src/arm64/instructions-arm64.cc +++ b/src/arm64/instructions-arm64.cc @@ -4,15 +4,12 @@ #if V8_TARGET_ARCH_ARM64 -#define ARM64_DEFINE_FP_STATICS - #include "src/arm64/assembler-arm64-inl.h" #include "src/arm64/instructions-arm64.h" namespace v8 { namespace internal { - bool Instruction::IsLoad() const { if (Mask(LoadStoreAnyFMask) != LoadStoreAnyFixed) { return false; diff --git a/src/arm64/instructions-arm64.h b/src/arm64/instructions-arm64.h index b6b38166bf..0c59a425cc 100644 --- a/src/arm64/instructions-arm64.h +++ b/src/arm64/instructions-arm64.h @@ -18,44 +18,26 @@ namespace internal { typedef uint32_t Instr; -// The following macros initialize a float/double variable with a bit pattern -// without using static initializers: If ARM64_DEFINE_FP_STATICS is defined, the -// symbol is defined as uint32_t/uint64_t initialized with the desired bit -// pattern. Otherwise, the same symbol is declared as an external float/double. -#if defined(ARM64_DEFINE_FP_STATICS) -#define DEFINE_FLOAT16(name, value) extern const uint16_t name = value -#define DEFINE_FLOAT(name, value) extern const uint32_t name = value -#define DEFINE_DOUBLE(name, value) extern const uint64_t name = value -#else -#define DEFINE_FLOAT16(name, value) extern const float16 name -#define DEFINE_FLOAT(name, value) extern const float name -#define DEFINE_DOUBLE(name, value) extern const double name -#endif // defined(ARM64_DEFINE_FP_STATICS) - -DEFINE_FLOAT16(kFP16PositiveInfinity, 0x7c00); -DEFINE_FLOAT16(kFP16NegativeInfinity, 0xfc00); -DEFINE_FLOAT(kFP32PositiveInfinity, 0x7f800000); -DEFINE_FLOAT(kFP32NegativeInfinity, 0xff800000); -DEFINE_DOUBLE(kFP64PositiveInfinity, 0x7ff0000000000000UL); -DEFINE_DOUBLE(kFP64NegativeInfinity, 0xfff0000000000000UL); +extern const float16 kFP16PositiveInfinity; +extern const float16 kFP16NegativeInfinity; +extern const float kFP32PositiveInfinity; +extern const float kFP32NegativeInfinity; +extern const double kFP64PositiveInfinity; +extern const double kFP64NegativeInfinity; // This value is a signalling NaN as both a double and as a float (taking the // least-significant word). -DEFINE_DOUBLE(kFP64SignallingNaN, 0x7ff000007f800001); -DEFINE_FLOAT(kFP32SignallingNaN, 0x7f800001); +extern const double kFP64SignallingNaN; +extern const float kFP32SignallingNaN; // A similar value, but as a quiet NaN. -DEFINE_DOUBLE(kFP64QuietNaN, 0x7ff800007fc00001); -DEFINE_FLOAT(kFP32QuietNaN, 0x7fc00001); +extern const double kFP64QuietNaN; +extern const float kFP32QuietNaN; // The default NaN values (for FPCR.DN=1). -DEFINE_DOUBLE(kFP64DefaultNaN, 0x7ff8000000000000UL); -DEFINE_FLOAT(kFP32DefaultNaN, 0x7fc00000); -DEFINE_FLOAT16(kFP16DefaultNaN, 0x7e00); - -#undef DEFINE_FLOAT16 -#undef DEFINE_FLOAT -#undef DEFINE_DOUBLE +extern const double kFP64DefaultNaN; +extern const float kFP32DefaultNaN; +extern const float16 kFP16DefaultNaN; unsigned CalcLSDataSize(LoadStoreOp op); unsigned CalcLSPairDataSize(LoadStorePairOp op); diff --git a/src/v8.gyp b/src/v8.gyp index 736fd48c6c..f3accd524d 100644 --- a/src/v8.gyp +++ b/src/v8.gyp @@ -1540,6 +1540,7 @@ 'arm64/disasm-arm64.h', 'arm64/frame-constants-arm64.cc', 'arm64/frame-constants-arm64.h', + 'arm64/instructions-arm64-constants.cc', 'arm64/instructions-arm64.cc', 'arm64/instructions-arm64.h', 'arm64/instrument-arm64.cc',