Bug 1530937 part 4 - Move pointer to C++ function out of VMFunctionData. r=nbp
authorJan de Mooij <jdemooij@mozilla.com>
Mon, 04 Mar 2019 14:40:45 +0000
changeset 520106 77b7364aabec18d52b6f520c14ef4ec21c614600
parent 520105 3702a10f9758ce8e45350a24789c6e2d3a15b743
child 520107 f9252c02242cd4599c6648c1ef375f76d699d40b
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnbp
bugs1530937
milestone67.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 1530937 part 4 - Move pointer to C++ function out of VMFunctionData. r=nbp GCC 8.2/8.3 doesn't like the (void*) trick in (some) constexpr contexts because it treats it like a reinterpret_cast (which isn't allowed in constexpr). This patch moves the function pointers into a separate const array so that VMFunctionData can stay constexpr and relies on compilers being smart enough to treat the function-pointer-array as constexpr. Differential Revision: https://phabricator.services.mozilla.com/D21922
js/src/jit/Ion.cpp
js/src/jit/JitRealm.h
js/src/jit/VMFunctions.cpp
js/src/jit/VMFunctions.h
js/src/jit/arm/Trampoline-arm.cpp
js/src/jit/arm64/Trampoline-arm64.cpp
js/src/jit/none/Trampoline-none.cpp
js/src/jit/x64/Trampoline-x64.cpp
js/src/jit/x86/Trampoline-x86.cpp
--- a/js/src/jit/Ion.cpp
+++ b/js/src/jit/Ion.cpp
@@ -293,17 +293,17 @@ bool JitRuntime::initialize(JSContext* c
   // TODO(bug 1530937): remove this after converting all VM functions.
   for (VMFunction* fun = VMFunction::functions; fun; fun = fun->next) {
     if (functionWrappers_->has(fun)) {
       // Duplicate VMFunction definition. See VMFunction::hash.
       continue;
     }
     JitSpew(JitSpew_Codegen, "# VM function wrapper (%s)", fun->name());
     uint32_t offset;
-    if (!generateVMWrapper(cx, masm, *fun, &offset)) {
+    if (!generateVMWrapper(cx, masm, *fun, fun->wrapped, &offset)) {
       return false;
     }
     if (!functionWrappers_->putNew(fun, offset)) {
       return false;
     }
   }
 
   JitSpew(JitSpew_Codegen, "# Emitting profiler exit frame tail stub");
--- a/js/src/jit/JitRealm.h
+++ b/js/src/jit/JitRealm.h
@@ -193,17 +193,18 @@ class JitRuntime {
                               MIRType type);
   void generateMallocStub(MacroAssembler& masm);
   void generateFreeStub(MacroAssembler& masm);
   JitCode* generateDebugTrapHandler(JSContext* cx);
   JitCode* generateBaselineDebugModeOSRHandler(
       JSContext* cx, uint32_t* noFrameRegPopOffsetOut);
 
   bool generateVMWrapper(JSContext* cx, MacroAssembler& masm,
-                         const VMFunctionData& f, uint32_t* wrapperOffset);
+                         const VMFunctionData& f, void* nativeFun,
+                         uint32_t* wrapperOffset);
   bool generateVMWrappers(JSContext* cx, MacroAssembler& masm);
 
   bool generateTLEventVM(MacroAssembler& masm, const VMFunctionData& f,
                          bool enter);
 
   inline bool generateTLEnterVM(MacroAssembler& masm, const VMFunctionData& f) {
     return generateTLEventVM(masm, f, /* enter = */ true);
   }
--- a/js/src/jit/VMFunctions.cpp
+++ b/js/src/jit/VMFunctions.cpp
@@ -62,50 +62,59 @@ struct VMFunctionDataHelper<R (*)(JSCont
     return BitMask<TypeToArgProperties, uint32_t, 2, Args...>::result;
   }
   static constexpr uint32_t argumentPassedInFloatRegs() {
     return BitMask<TypeToPassInFloatReg, uint32_t, 2, Args...>::result;
   }
   static constexpr uint64_t argumentRootTypes() {
     return BitMask<TypeToRootType, uint64_t, 3, Args...>::result;
   }
-  constexpr VMFunctionDataHelper(Fun fun, const char* name,
-                                 PopValues extraValuesToPop = PopValues(0))
-      : VMFunctionData((void*)fun, name, explicitArgs(), argumentProperties(),
+  constexpr explicit VMFunctionDataHelper(
+      const char* name, PopValues extraValuesToPop = PopValues(0))
+      : VMFunctionData(name, explicitArgs(), argumentProperties(),
                        argumentPassedInFloatRegs(), argumentRootTypes(),
                        outParam(), outParamRootType(), returnType(),
                        extraValuesToPop.numValues, NonTailCall) {}
-  constexpr VMFunctionDataHelper(Fun fun, const char* name,
-                                 MaybeTailCall expectTailCall,
-                                 PopValues extraValuesToPop = PopValues(0))
-      : VMFunctionData((void*)fun, name, explicitArgs(), argumentProperties(),
+  constexpr explicit VMFunctionDataHelper(
+      const char* name, MaybeTailCall expectTailCall,
+      PopValues extraValuesToPop = PopValues(0))
+      : VMFunctionData(name, explicitArgs(), argumentProperties(),
                        argumentPassedInFloatRegs(), argumentRootTypes(),
                        outParam(), outParamRootType(), returnType(),
                        extraValuesToPop.numValues, expectTailCall) {}
 };
 
 // GCC warns when the signature does not have matching attributes (for example
 // MOZ_MUST_USE). Squelch this warning to avoid a GCC-only footgun.
 #if MOZ_IS_GCC
 #  pragma GCC diagnostic push
 #  pragma GCC diagnostic ignored "-Wignored-attributes"
 #endif
 
 // Generate VMFunctionData array.
 static constexpr VMFunctionData vmFunctions[] = {
-#define DEF_VMFUNCTION(name, fp) \
-  VMFunctionDataHelper<decltype(&(::fp))>(::fp, #name),
+#define DEF_VMFUNCTION(name, fp) VMFunctionDataHelper<decltype(&(::fp))>(#name),
     VMFUNCTION_LIST(DEF_VMFUNCTION)
 #undef DEF_VMFUNCTION
 };
 
 #if MOZ_IS_GCC
 #  pragma GCC diagnostic pop
 #endif
 
+// Generate array storing C++ function pointers. These pointers are not stored
+// in VMFunctionData because there's no good way to cast them to void* in
+// constexpr code. Compilers are smart enough to treat the const array below as
+// constexpr.
+static void* const vmFunctionTargets[] = {
+#define DEF_VMFUNCTION(name, fp) (void*)(::fp),
+    VMFUNCTION_LIST(DEF_VMFUNCTION)
+#undef DEF_VMFUNCTION
+};
+
 const VMFunctionData& GetVMFunction(VMFunctionId id) {
   return vmFunctions[size_t(id)];
 }
 
 bool JitRuntime::generateVMWrappers(JSContext* cx, MacroAssembler& masm) {
   // Generate all VM function wrappers.
 
   static constexpr size_t NumVMFunctions = size_t(VMFunctionId::Count);
@@ -129,17 +138,17 @@ bool JitRuntime::generateVMWrappers(JSCo
                  "VM function list must be sorted by name");
     }
     lastName = fun.name();
 #endif
 
     JitSpew(JitSpew_Codegen, "# VM function wrapper (%s)", fun.name());
 
     uint32_t offset;
-    if (!generateVMWrapper(cx, masm, fun, &offset)) {
+    if (!generateVMWrapper(cx, masm, fun, vmFunctionTargets[i], &offset)) {
       return false;
     }
 
     MOZ_ASSERT(functionWrapperOffsets_.length() == size_t(id));
     functionWrapperOffsets_.infallibleAppend(offset);
   }
 
   return true;
--- a/js/src/jit/VMFunctions.h
+++ b/js/src/jit/VMFunctions.h
@@ -123,19 +123,16 @@ enum MaybeTailCall : bool { TailCall, No
 //      if (!callVM(FooInfo)) {
 //          return false;
 //      }
 //
 // After this, the result value is in the return value register.
 
 // Data for a VM function. All VMFunctionDatas are stored in a constexpr array.
 struct VMFunctionData {
-  // Address of the C function.
-  void* wrapped;
-
 #if defined(JS_JITSPEW) || defined(JS_TRACE_LOGGING)
   // Informative name of the wrapped function. The name should not be present
   // in release builds in order to save memory.
   const char* name_;
 #endif
 
   // Note: a maximum of seven root types is supported.
   enum RootType : uint8_t {
@@ -288,24 +285,24 @@ struct VMFunctionData {
     // few loop iterations)
     while (n) {
       count++;
       n &= n - 1;
     }
     return count;
   }
 
-  constexpr VMFunctionData(void* wrapped, const char* name,
-                           uint32_t explicitArgs, uint32_t argumentProperties,
+  constexpr VMFunctionData(const char* name, uint32_t explicitArgs,
+                           uint32_t argumentProperties,
                            uint32_t argumentPassedInFloatRegs,
                            uint64_t argRootTypes, DataType outParam,
                            RootType outParamRootType, DataType returnType,
                            uint8_t extraValuesToPop = 0,
                            MaybeTailCall expectTailCall = NonTailCall)
-      : wrapped(wrapped),
+      :
 #if defined(JS_JITSPEW) || defined(JS_TRACE_LOGGING)
         name_(name),
 #endif
         argumentRootTypes(argRootTypes),
         argumentProperties(argumentProperties),
         argumentPassedInFloatRegs(argumentPassedInFloatRegs),
         explicitArgs(explicitArgs),
         outParamRootType(outParamRootType),
@@ -319,17 +316,17 @@ struct VMFunctionData {
     MOZ_ASSERT(returnType == Type_Void || returnType == Type_Bool ||
                returnType == Type_Object);
   }
 
   // Note: clang-tidy suggests using |= auto| here but that generates extra
   // static initializers for old-style VMFunction definitions with Clang. We can
   // do this after bug 1530937 converts all of them.
   constexpr VMFunctionData(const VMFunctionData& o)
-      : wrapped(o.wrapped),
+      :
 #if defined(JS_JITSPEW) || defined(JS_TRACE_LOGGING)
         name_(o.name_),
 #endif
         argumentRootTypes(o.argumentRootTypes),
         argumentProperties(o.argumentProperties),
         argumentPassedInFloatRegs(o.argumentPassedInFloatRegs),
         explicitArgs(o.explicitArgs),
         outParamRootType(o.outParamRootType),
@@ -338,34 +335,39 @@ struct VMFunctionData {
         extraValuesToPop(o.extraValuesToPop),
         expectTailCall(o.expectTailCall) {
   }
 };
 
 // TODO(bug 1530937): remove VMFunction and FunctionInfo after converting all VM
 // functions to the new design.
 struct VMFunction : public VMFunctionData {
+  // Address of the C function.
+  void* wrapped;
+
   // Global linked list of all VMFunctions.
   static VMFunction* functions;
   VMFunction* next;
 
   constexpr VMFunction(void* wrapped, const char* name, uint32_t explicitArgs,
                        uint32_t argumentProperties,
                        uint32_t argumentPassedInFloatRegs,
                        uint64_t argRootTypes, DataType outParam,
                        RootType outParamRootType, DataType returnType,
                        uint8_t extraValuesToPop = 0,
                        MaybeTailCall expectTailCall = NonTailCall)
-      : VMFunctionData(wrapped, name, explicitArgs, argumentProperties,
+      : VMFunctionData(name, explicitArgs, argumentProperties,
                        argumentPassedInFloatRegs, argRootTypes, outParam,
                        outParamRootType, returnType, extraValuesToPop,
                        expectTailCall),
+        wrapped(wrapped),
         next(nullptr) {}
 
-  VMFunction(const VMFunction& o) : VMFunctionData(o), next(functions) {
+  VMFunction(const VMFunction& o)
+      : VMFunctionData(o), wrapped(o.wrapped), next(functions) {
     // Add this to the global list of VMFunctions.
     functions = this;
   }
 
   typedef const VMFunction* Lookup;
 
   static HashNumber hash(const VMFunction* f) {
     // The hash is based on the wrapped function, not the VMFunction*, to
--- a/js/src/jit/arm/Trampoline-arm.cpp
+++ b/js/src/jit/arm/Trampoline-arm.cpp
@@ -699,17 +699,17 @@ JitRuntime::BailoutTable JitRuntime::gen
 void JitRuntime::generateBailoutHandler(MacroAssembler& masm,
                                         Label* bailoutTail) {
   bailoutHandlerOffset_ = startTrampolineCode(masm);
 
   GenerateBailoutThunk(masm, NO_FRAME_SIZE_CLASS_ID, bailoutTail);
 }
 
 bool JitRuntime::generateVMWrapper(JSContext* cx, MacroAssembler& masm,
-                                   const VMFunctionData& f,
+                                   const VMFunctionData& f, void* nativeFun,
                                    uint32_t* wrapperOffset) {
   MOZ_ASSERT(functionWrappers_);
 
   *wrapperOffset = startTrampolineCode(masm);
 
   AllocatableGeneralRegisterSet regs(Register::Codes::WrapperMask);
 
   static_assert(
@@ -820,17 +820,17 @@ bool JitRuntime::generateVMWrapper(JSCon
     }
   }
 
   // Copy the implicit outparam, if any.
   if (outReg != InvalidReg) {
     masm.passABIArg(outReg);
   }
 
-  masm.callWithABI(f.wrapped, MoveOp::GENERAL,
+  masm.callWithABI(nativeFun, MoveOp::GENERAL,
                    CheckUnsafeCallWithABI::DontCheckHasExitFrame);
 
   if (!generateTLExitVM(masm, f)) {
     return false;
   }
 
   // Test for failure.
   switch (f.failType()) {
--- a/js/src/jit/arm64/Trampoline-arm64.cpp
+++ b/js/src/jit/arm64/Trampoline-arm64.cpp
@@ -528,17 +528,17 @@ JitRuntime::BailoutTable JitRuntime::gen
 void JitRuntime::generateBailoutHandler(MacroAssembler& masm,
                                         Label* bailoutTail) {
   bailoutHandlerOffset_ = startTrampolineCode(masm);
 
   GenerateBailoutThunk(masm, bailoutTail);
 }
 
 bool JitRuntime::generateVMWrapper(JSContext* cx, MacroAssembler& masm,
-                                   const VMFunctionData& f,
+                                   const VMFunctionData& f, void* nativeFun,
                                    uint32_t* wrapperOffset) {
   MOZ_ASSERT(functionWrappers_);
 
   *wrapperOffset = startTrampolineCode(masm);
 
   // Avoid conflicts with argument registers while discarding the result after
   // the function call.
   AllocatableGeneralRegisterSet regs(Register::Codes::WrapperMask);
@@ -652,17 +652,17 @@ bool JitRuntime::generateVMWrapper(JSCon
   // Copy the semi-implicit outparam, if any.
   // It is not a C++-abi outparam, which would get passed in the
   // outparam register, but a real parameter to the function, which
   // was stack-allocated above.
   if (outReg != InvalidReg) {
     masm.passABIArg(outReg);
   }
 
-  masm.callWithABI(f.wrapped, MoveOp::GENERAL,
+  masm.callWithABI(nativeFun, MoveOp::GENERAL,
                    CheckUnsafeCallWithABI::DontCheckHasExitFrame);
 
   if (!generateTLExitVM(masm, f)) {
     return false;
   }
 
   // SP is used to transfer stack across call boundaries.
   masm.initPseudoStackPtr();
--- a/js/src/jit/none/Trampoline-none.cpp
+++ b/js/src/jit/none/Trampoline-none.cpp
@@ -35,17 +35,17 @@ void JitRuntime::generateExceptionTailSt
 void JitRuntime::generateBailoutTailStub(MacroAssembler&, Label*) {
   MOZ_CRASH();
 }
 void JitRuntime::generateProfilerExitFrameTailStub(MacroAssembler&, Label*) {
   MOZ_CRASH();
 }
 
 bool JitRuntime::generateVMWrapper(JSContext*, MacroAssembler&,
-                                   const VMFunctionData&, uint32_t*) {
+                                   const VMFunctionData&, void*, uint32_t*) {
   MOZ_CRASH();
 }
 
 FrameSizeClass FrameSizeClass::FromDepth(uint32_t) { MOZ_CRASH(); }
 FrameSizeClass FrameSizeClass::ClassLimit() { MOZ_CRASH(); }
 uint32_t FrameSizeClass::frameSize() const { MOZ_CRASH(); }
 
 BailoutFrameInfo::BailoutFrameInfo(const JitActivationIterator& iter,
--- a/js/src/jit/x64/Trampoline-x64.cpp
+++ b/js/src/jit/x64/Trampoline-x64.cpp
@@ -586,17 +586,17 @@ JitRuntime::BailoutTable JitRuntime::gen
 void JitRuntime::generateBailoutHandler(MacroAssembler& masm,
                                         Label* bailoutTail) {
   bailoutHandlerOffset_ = startTrampolineCode(masm);
 
   GenerateBailoutThunk(masm, NO_FRAME_SIZE_CLASS_ID, bailoutTail);
 }
 
 bool JitRuntime::generateVMWrapper(JSContext* cx, MacroAssembler& masm,
-                                   const VMFunctionData& f,
+                                   const VMFunctionData& f, void* nativeFun,
                                    uint32_t* wrapperOffset) {
   MOZ_ASSERT(functionWrappers_);
 
   *wrapperOffset = startTrampolineCode(masm);
 
   // Avoid conflicts with argument registers while discarding the result after
   // the function call.
   AllocatableGeneralRegisterSet regs(Register::Codes::WrapperMask);
@@ -698,17 +698,17 @@ bool JitRuntime::generateVMWrapper(JSCon
     }
   }
 
   // Copy the implicit outparam, if any.
   if (outReg != InvalidReg) {
     masm.passABIArg(outReg);
   }
 
-  masm.callWithABI(f.wrapped, MoveOp::GENERAL,
+  masm.callWithABI(nativeFun, MoveOp::GENERAL,
                    CheckUnsafeCallWithABI::DontCheckHasExitFrame);
 
   if (!generateTLExitVM(masm, f)) {
     return false;
   }
 
   // Test for failure.
   switch (f.failType()) {
--- a/js/src/jit/x86/Trampoline-x86.cpp
+++ b/js/src/jit/x86/Trampoline-x86.cpp
@@ -603,17 +603,17 @@ JitRuntime::BailoutTable JitRuntime::gen
 void JitRuntime::generateBailoutHandler(MacroAssembler& masm,
                                         Label* bailoutTail) {
   bailoutHandlerOffset_ = startTrampolineCode(masm);
 
   GenerateBailoutThunk(masm, NO_FRAME_SIZE_CLASS_ID, bailoutTail);
 }
 
 bool JitRuntime::generateVMWrapper(JSContext* cx, MacroAssembler& masm,
-                                   const VMFunctionData& f,
+                                   const VMFunctionData& f, void* nativeFun,
                                    uint32_t* wrapperOffset) {
   MOZ_ASSERT(functionWrappers_);
 
   *wrapperOffset = startTrampolineCode(masm);
 
   // Avoid conflicts with argument registers while discarding the result after
   // the function call.
   AllocatableGeneralRegisterSet regs(Register::Codes::WrapperMask);
@@ -715,17 +715,17 @@ bool JitRuntime::generateVMWrapper(JSCon
     }
   }
 
   // Copy the implicit outparam, if any.
   if (outReg != InvalidReg) {
     masm.passABIArg(outReg);
   }
 
-  masm.callWithABI(f.wrapped, MoveOp::GENERAL,
+  masm.callWithABI(nativeFun, MoveOp::GENERAL,
                    CheckUnsafeCallWithABI::DontCheckHasExitFrame);
 
   if (!generateTLExitVM(masm, f)) {
     return false;
   }
 
   // Test for failure.
   switch (f.failType()) {