Bug 155178

Created attachment 273309 [details] Patch

Waiting for builds

Comment on attachment 273309 [details] Patch looks good to me so far. lets see what happens with the bots.

Created attachment 273311 [details] Patch

(In reply to comment #4) > Created attachment 273311 [details] > Patch because being able to build is totes overrated :D

Committed r197793: <http://trac.webkit.org/changeset/197793>

build fix in r197799

(In reply to comment #7) > build fix in r197799 ... :-/ I'm going to assume Xcode here. I'm just going to verify that the patch otherwise matched what was reviewed :-/

(In reply to comment #8) > (In reply to comment #7) > > build fix in r197799 > > > ... > > :-/ > > I'm going to assume Xcode here. I'm just going to verify that the patch > otherwise matched what was reviewed :-/ Seriously confused by what Xcode did to the patch post review. I'm going to rollout this (and your subsequent patch) and get the bot to re-land it, to be safe because the attached patch is the correct one.

Comment on attachment 273311 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=273311&action=review if you're going to reland it, please remember these fixes I also needed: > Source/JavaScriptCore/jit/ExecutableAllocatorFixedVMPool.cpp:152 > + VM_FLAGS_ANYWHERE | VM_FLAGS_RANDOM_ADDR, VM_FLAGS_RANDOM_ADDR is not defined in my sdk. does not build. > Source/JavaScriptCore/jit/ExecutableAllocatorFixedVMPool.cpp:164 > + int result = mprotect(stubBase, stubSize, VM_PROT_EXECUTE_ONLY); result is not defined when VM_PROT_EXECUTE_ONLY is not defined. Does not build.

Re-opened since this is blocked by bug 155195

Comment on attachment 273311 [details] Patch Something bizarre happened when this got landed. Happily? it did show another issue i missed

Created attachment 273355 [details] All the build fixes for non-separated heaps

Created attachment 273356 [details] All the build fixes for both at the same time. sigh.

Committed r197816: <http://trac.webkit.org/changeset/197816>

This broke iOS build: In file included from /Volumes/Data/slave/ios-9-release/build/Source/JavaScriptCore/jit/ExecutableAllocatorFixedVMPool.cpp:57: /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS9.0.sdk/usr/include/mach/mach_vm.h:1:2: error: mach_vm.h unsupported. #error mach_vm.h unsupported. ^ /Volumes/Data/slave/ios-9-release/build/Source/JavaScriptCore/jit/ExecutableAllocatorFixedVMPool.cpp:156:33: error: use of undeclared identifier 'VM_FLAGS_RANDOM_ADDR' VM_FLAGS_ANYWHERE | VM_FLAGS_RANDOM_ADDR,

(In reply to comment #15) > Committed r197816: <http://trac.webkit.org/changeset/197816> It broke the WinCairo build too: https://build.webkit.org/builders/WinCairo%2064-Bit%20Release/builds/54752

Comment on attachment 273356 [details] All the build fixes for both at the same time. sigh. View in context: https://bugs.webkit.org/attachment.cgi?id=273356&action=review > Source/JavaScriptCore/assembler/MacroAssemblerARM64.h:1271 > + m_assembler.stp<64>(src1, src2, dest, offset.m_value); Can you please update your storePair/loadPair to use Address instead of base+immediate? This API is confusing IMHO. It look like you are exposing STP/STNP directly but you are not. The offset is scaled. With Address it is clear: the offset is always in bytes.

(In reply to comment #18) > Comment on attachment 273356 [details] > All the build fixes for both at the same time. sigh. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=273356&action=review > > > Source/JavaScriptCore/assembler/MacroAssemblerARM64.h:1271 > > + m_assembler.stp<64>(src1, src2, dest, offset.m_value); > > Can you please update your storePair/loadPair to use Address instead of > base+immediate? > > This API is confusing IMHO. It look like you are exposing STP/STNP directly > but you are not. The offset is scaled. > With Address it is clear: the offset is always in bytes. Will do.