mozilla-central: changeset 319003:37cff35583fe1405d6466280aa70681df8aff3a4

author	nfroyd@mozilla.com <nfroyd@mozilla.com>
	Fri, 21 Oct 2016 23:06:47 +0000
changeset 319003	37cff35583fe1405d6466280aa70681df8aff3a4
parent 318938	806054dd12bdcbdee81dbd75f1583156cef9b649
child 319004	5118162c1a82afa6702fb1040f2050917f6ccc65
push id	30858
push user	ryanvm@gmail.com
push date	Sun, 23 Oct 2016 17:17:41 +0000
treeherder	mozilla-central@a9a41b69f3f9 [default view] [failures only]
perfherder	[talos] [build metrics] [platform microbench] (compared to previous push)
reviewers	jandem
bugs	1305816
milestone	52.0a1
first release with	nightly linux32 215f96861176 / 52.0a1 / 20161024030205 / files nightly linux64 215f96861176 / 52.0a1 / 20161024030205 / files nightly mac 215f96861176 / 52.0a1 / 20161024030205 / files nightly win32 215f96861176 / 52.0a1 / 20161024030205 / files nightly win64 215f96861176 / 52.0a1 / 20161024030205 / files
last release without	nightly linux32 f0f1aaf051d6 / 52.0a1 / 20161021030210 / files nightly linux64 f0f1aaf051d6 / 52.0a1 / 20161021030210 / files nightly mac f0f1aaf051d6 / 52.0a1 / 20161021030210 / files nightly win32 f0f1aaf051d6 / 52.0a1 / 20161021030210 / files nightly win64 f0f1aaf051d6 / 52.0a1 / 20161021030210 / files

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.

--- 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 {

js/src/jit/x86-shared/Assembler-x86-shared.h		file \| annotate \| diff \| comparison \| revisions
js/src/jit/x86-shared/Constants-x86-shared.h		file \| annotate \| diff \| comparison \| revisions