Bug 1305816 - explicitly specify underlying type of js::jit::X86Encoding::RegisterID; r=jandem
authornfroyd@mozilla.com <nfroyd@mozilla.com>
Fri, 21 Oct 2016 23:06:47 +0000
changeset 319003 37cff35583fe1405d6466280aa70681df8aff3a4
parent 318938 806054dd12bdcbdee81dbd75f1583156cef9b649
child 319004 5118162c1a82afa6702fb1040f2050917f6ccc65
push id30858
push userryanvm@gmail.com
push dateSun, 23 Oct 2016 17:17:41 +0000
treeherdermozilla-central@a9a41b69f3f9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1305816
milestone52.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1305816 - explicitly specify underlying type of js::jit::X86Encoding::RegisterID; r=jandem Recent clang-cl warns when building for x86-64 Windows about initializing Assembler-x86-shared.h's Operand::index_ with Registers::Invalid. This warning stems from a couple of implementation-defined behaviors in the C++ standard: - The only constraints on an (non-fixed-width) enum's type are that the type is at least as large as `int`, and that the chosen type can accomodate all the values of the enum. MSVC (and clang-cl) default to int wherever possible. - Bitfields declared with `int` (resp. `char`, `short`, `long`) may be either signed or unsigned at the implementation's discretion. It is therefore encouraged that you always declare bitfields with unsigned types. Operand::index_ is a 5-bit field, with a declared type of Register::Encoding; on x86, that boils down to the enum js::jit::X86Encoding::RegisterID. The compiler defaults the underlying type of RegisterID to `int` (so the bitfield is signed on some implementations), and Registers::Invalid is 16 on x86-64, large enough to silently change sign when stored into the bitfield. Declaring the bitfield as a fixed-width unsigned 8-bit enum is sufficient to silence the warning.
js/src/jit/x86-shared/Assembler-x86-shared.h
js/src/jit/x86-shared/Constants-x86-shared.h
--- a/js/src/jit/x86-shared/Assembler-x86-shared.h
+++ b/js/src/jit/x86-shared/Assembler-x86-shared.h
@@ -54,17 +54,19 @@ class Operand
         MEM_ADDRESS32
     };
 
   private:
     Kind kind_ : 4;
     // Used as a Register::Encoding and a FloatRegister::Encoding.
     uint32_t base_ : 5;
     Scale scale_ : 3;
-    Register::Encoding index_ : 5;
+    // We don't use all 8 bits, of course, but GCC complains if the size of
+    // this field is smaller than the size of Register::Encoding.
+    Register::Encoding index_ : 8;
     int32_t disp_;
 
   public:
     explicit Operand(Register reg)
       : kind_(REG),
         base_(reg.encoding()),
         scale_(TimesOne),
         index_(Registers::Invalid),
--- a/js/src/jit/x86-shared/Constants-x86-shared.h
+++ b/js/src/jit/x86-shared/Constants-x86-shared.h
@@ -6,23 +6,24 @@
 
 #ifndef jit_x86_shared_Constants_x86_shared_h
 #define jit_x86_shared_Constants_x86_shared_h
 
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/Assertions.h"
 
 #include <stddef.h>
+#include <stdint.h>
 
 namespace js {
 namespace jit {
 
 namespace X86Encoding {
 
-enum RegisterID {
+enum RegisterID : uint8_t {
     rax, rcx, rdx, rbx, rsp, rbp, rsi, rdi
 #ifdef JS_CODEGEN_X64
    ,r8, r9, r10, r11, r12, r13, r14, r15
 #endif
    ,invalid_reg
 };
 
 enum HRegisterID {