1774390 - Remove frame size and header size from frame descriptor

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, task, P3)

Description

[Jan de Mooij [:jandem]]

We can now use frame pointers instead. This is especially nice for Baseline (IC) code where we often had to compute the frame size dynamically for the descriptor.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Bug 1774390 part 1 - Use separate ExitFrameType for EnsureBareExitFrame. r?iain!

This will be used by a later patch, but it's also nice for assertions to use
a different type.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1774390 part 2 - Don't depend on frame descriptor size in getCppEntryRegisters. r?iain!

Depends on D149376

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1774390 part 3 - Remove frame size and header size from frame descriptor. r?iain!

This was used to find the previous frame on the stack, but we can now use frame pointers
for that.

Depends on D149377

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1774390 part 4 - Simplify emitApplyGeneric and related functions. r?iain!

Now that we no longer have to keep track of the number of arguments we pushed,
we can turn the extraStackSpace register into a scratch register.

Also simplify emitPushArguments more by using the frame pointer as base register
instead of a separate register.

Depends on D149378

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/45576a2dc383
part 1 - Use separate ExitFrameType for EnsureBareExitFrame. r=iain
https://hg.mozilla.org/integration/autoland/rev/e1792844b5b1
part 2 - Don't depend on frame descriptor size in getCppEntryRegisters. r=iain
https://hg.mozilla.org/integration/autoland/rev/6072f76850e7
part 3 - Remove frame size and header size from frame descriptor. r=iain
https://hg.mozilla.org/integration/autoland/rev/bdfcc8363de5
part 4 - Simplify emitApplyGeneric and related functions. r=iain

Comment 6

[bszekely]

Backed out for causing assertion failures on Trampoline-x64.cpp

Failure line: 8317> Assertion failure: enterJITStackEntry->rbp == fp, at /builds/worker/checkouts/gecko/js/src/jit/x64/Trampoline-x64.cpp:373

Push with failures

Failure log

Backout link

[task 2022-06-16T10:52:22.586Z] 10:52:22     INFO - TEST-START | toolkit/components/backgroundtasks/tests/browser/browser_xpcom_graph_wait.js
[task 2022-06-16T10:52:22.623Z] 10:52:22     INFO - GECKO(8038) | console.info: "launching background task" ({command:"/builds/worker/workspace/build/application/firefox/firefox-bin", args:["--backgroundtask", "wait"], extraEnv:{MOZ_PROFILER_STARTUP:"1", MOZ_PROFILER_SHUTDOWN:"/builds/worker/workspace/build/blobber_upload_dir/profile_backgroundtask_wait.json", XPCSHELL_TESTING_MODULES_URI:"file:////builds/worker/workspace/build/tests/modules/"}})
[task 2022-06-16T10:52:22.706Z] 10:52:22     INFO - GECKO(8038) | 8318> ### XPCOM_MEM_BLOAT_LOG defined -- logging bloat/leaks to /tmp/tmp_lwhxnsi.mozrunner/runtests_leaks.log
[task 2022-06-16T10:52:22.708Z] 10:52:22     INFO - GECKO(8038) | 8318> [8318, Main Thread] WARNING: XPCOM_MEM_BLOAT_LOG is set, disabling native allocations.: file /builds/worker/checkouts/gecko/tools/profiler/core/platform.cpp:339
[task 2022-06-16T10:52:22.709Z] 10:52:22     INFO - GECKO(8038) | 8318> [8318, Main Thread] WARNING: XPCOM_MEM_BLOAT_LOG is set, disabling native allocations.: file /builds/worker/checkouts/gecko/tools/profiler/core/platform.cpp:339
[task 2022-06-16T10:52:22.710Z] 10:52:22     INFO - GECKO(8038) | 8318> [8318, Main Thread] WARNING: XPCOM_MEM_BLOAT_LOG is set, disabling native allocations.: file /builds/worker/checkouts/gecko/tools/profiler/core/platform.cpp:339
[task 2022-06-16T10:52:24.543Z] 10:52:24     INFO - GECKO(8038) | 8318> *** You are running in background task mode. ***
[task 2022-06-16T10:52:24.544Z] 10:52:24     INFO - GECKO(8038) | 8318> *** You are running in headless mode.
[task 2022-06-16T10:52:24.550Z] 10:52:24     INFO - GECKO(8038) | 8318> [8318, Main Thread] WARNING: ShouldNotProcessUpdates(): NotAnUpdatingTask: file /builds/worker/checkouts/gecko/toolkit/xre/nsAppRunner.cpp:4585
[task 2022-06-16T10:52:24.755Z] 10:52:24     INFO - GECKO(8038) | 8318> Assertion failure: enterJITStackEntry->rbp == fp, at /builds/worker/checkouts/gecko/js/src/jit/x64/Trampoline-x64.cpp:373
[task 2022-06-16T10:52:24.768Z] 10:52:24     INFO -  Initializing stack-fixing for the first stack frame, this may take a while...
[task 2022-06-16T10:52:35.668Z] 10:52:35     INFO - GECKO(8038) | 8318> #01: js::jit::JitRuntime::getCppEntryRegisters(js::jit::JitFrameLayout*) [js/src/jit/x64/Trampoline-x64.cpp:373]
[task 2022-06-16T10:52:35.669Z] 10:52:35     INFO - GECKO(8038) | 8318> #02: JS::ProfilingFrameIterator::getCppEntryRegisters() const [js/src/vm/Stack.cpp:765]
[task 2022-06-16T10:52:35.671Z] 10:52:35     INFO - GECKO(8038) | 8318> #03: ExtractJsFrames(bool, mozilla::profiler::ThreadRegistrationUnlockedReaderAndAtomicRWOnThread const&, Registers const&, ProfilerStackCollector&, JS::ProfilingFrameIterator::Frame*, StackWalkControl*) [tools/profiler/core/platform.cpp:1752]
[task 2022-06-16T10:52:35.671Z] 10:52:35     INFO - GECKO(8038) | 8318> #04: DoSharedSample(bool, unsigned int, mozilla::profiler::ThreadRegistrationUnlockedReaderAndAtomicRWOnThread const&, JS::ProfilingFrameIterator::Frame*, Registers const&, unsigned long, unsigned long, ProfileBuffer&, mozilla::StackCaptureOptions) [tools/profiler/core/platform.cpp:2421]
[task 2022-06-16T10:52:35.672Z] 10:52:35     INFO - GECKO(8038) | 8318> #05: SamplerThread::Run() [tools/profiler/core/platform.cpp:4056]
[task 2022-06-16T10:52:35.672Z] 10:52:35     INFO - GECKO(8038) | 8318> #06: ThreadEntry(void*) [tools/profiler/core/platform-linux-android.cpp:506]
[task 2022-06-16T10:52:35.712Z] 10:52:35     INFO - GECKO(8038) | 8318> #07: set_alt_signal_stack_and_start(PthreadCreateParams*) [toolkit/crashreporter/pthread_create_interposer/pthread_create_interposer.cpp:80]
[task 2022-06-16T10:52:35.713Z] 10:52:35     INFO - GECKO(8038) | 8318> #08: ??? [/lib/x86_64-linux-gnu/libpthread.so.0 + 0x76db]
[task 2022-06-16T10:52:35.719Z] 10:52:35     INFO - GECKO(8038) | 8318> #09: clone [/lib/x86_64-linux-gnu/libc.so.6 + 0x121a3f]
[task 2022-06-16T10:52:35.720Z] 10:52:35     INFO - GECKO(8038) | 8318> #10: ??? (???:???)
[task 2022-06-16T10:52:35.720Z] 10:52:35     INFO - GECKO(8038) | 8318> ExceptionHandler::GenerateDump cloned child 8341
[task 2022-06-16T10:52:35.720Z] 10:52:35     INFO - GECKO(8038) | 8318> ExceptionHandler::SendContinueSignalToChild sent continue signal to child
[task 2022-06-16T10:52:35.720Z] 10:52:35     INFO - GECKO(8038) | 8318> ExceptionHandler::WaitForContinueSignal waiting for continue signal...
[task 2022-06-16T10:52:35.721Z] 10:52:35     INFO - TEST-INFO | started process screentopng
[task 2022-06-16T10:52:35.929Z] 10:52:35     INFO - TEST-INFO | screentopng: exit 0
[task 2022-06-16T10:52:35.930Z] 10:52:35     INFO - Buffered messages logged at 10:52:22
[task 2022-06-16T10:52:35.930Z] 10:52:35     INFO - Entering test bound test_xpcom_graph_wait
[task 2022-06-16T10:52:35.931Z] 10:52:35     INFO - TEST-PASS | toolkit/components/backgroundtasks/tests/browser/browser_xpcom_graph_wait.js | resource://testing-common is not substituted - true == true - 
[task 2022-06-16T10:52:35.931Z] 10:52:35     INFO - Buffered messages finished
[task 2022-06-16T10:52:35.932Z] 10:52:35     INFO - TEST-UNEXPECTED-FAIL | toolkit/components/backgroundtasks/tests/browser/browser_xpcom_graph_wait.js | 0 == 11 - JS frame :: chrome://mochitests/content/browser/toolkit/components/backgroundtasks/tests/browser/browser_xpcom_graph_wait.js :: test_xpcom_graph_wait :: line 217
[task 2022-06-16T10:52:35.932Z] 10:52:35     INFO - Stack trace:
[task 2022-06-16T10:52:35.933Z] 10:52:35     INFO - chrome://mochitests/content/browser/toolkit/components/backgroundtasks/tests/browser/browser_xpcom_graph_wait.js:test_xpcom_graph_wait:217
[task 2022-06-16T10:52:35.933Z] 10:52:35     INFO - Not taking screenshot here: see the one that was previously logged
[task 2022-06-16T10:52:35.934Z] 10:52:35     INFO - TEST-UNEXPECTED-FAIL | toolkit/components/backgroundtasks/tests/browser/browser_xpcom_graph_wait.js | Uncaught exception in test - NotFoundError: Could not open the file at /builds/worker/workspace/build/blobber_upload_dir/profile_backgroundtask_wait.json
[task 2022-06-16T10:52:35.934Z] 10:52:35     INFO - Leaving test bound test_xpcom_graph_wait
[task 2022-06-16T10:52:35.935Z] 10:52:35     INFO - GECKO(8038) | MEMORY STAT | vsize 11186MB | residentFast 430MB | heapAllocated 188MB
[task 2022-06-16T10:52:35.935Z] 10:52:35     INFO - TEST-OK | toolkit/components/backgroundtasks/tests/browser/browser_xpcom_graph_wait.js | took 2223ms

Comment 7

[Jan de Mooij [:jandem]]

That enterJITStackEntry->rbp == fp assertion failure is me being dumb: rbp is the frame pointer we saved and not fp, the one we set in the trampoline. I just deleted the assertion and verified locally that fixes the test + has a reasonable value for enterJITStackEntry->rbp.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e96eb5c2a43d
part 1 - Use separate ExitFrameType for EnsureBareExitFrame. r=iain
https://hg.mozilla.org/integration/autoland/rev/5f73c05c009d
part 2 - Don't depend on frame descriptor size in getCppEntryRegisters. r=iain
https://hg.mozilla.org/integration/autoland/rev/b8a23e39579a
part 3 - Remove frame size and header size from frame descriptor. r=iain
https://hg.mozilla.org/integration/autoland/rev/d55911bfc3da
part 4 - Simplify emitApplyGeneric and related functions. r=iain

Comment 9

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/e96eb5c2a43d
https://hg.mozilla.org/mozilla-central/rev/5f73c05c009d
https://hg.mozilla.org/mozilla-central/rev/b8a23e39579a
https://hg.mozilla.org/mozilla-central/rev/d55911bfc3da