We were using an unnecessary indirect call when we can be using a direct call.
Created attachment 436642 [details]
Created attachment 436643 [details]
Comment on attachment 436643 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=436643&action=review
> + For MacroAssemblerX86Common.cpp, we left the X86 and MSVC implementations unchanged.
> + For X86, I don't know the stack alignment requirements (if any) plus we might want
> + to delete this code eventually since we're not supporting the X86 JIT anymore.
> -static_assert(JIT_PROBE_EXECUTOR_PTR_TAG == JITProbeExecutorPtrTag);
Let's remove JITProbeExecutorPtrTag from JSCPtrTag.h since it is no longer used.
(In reply to Yusuke Suzuki from comment #4)
> > -static_assert(JIT_PROBE_EXECUTOR_PTR_TAG == JITProbeExecutorPtrTag);
> Let's remove JITProbeExecutorPtrTag from JSCPtrTag.h since it is no longer
Ah yes, I had planned to do that but forgot.
Created attachment 436647 [details]
Comment on attachment 436647 [details]
Thanks for the review. Landed in r281718: <http://trac.webkit.org/r281718>.
This patch has a bug (accidentally deleted 2 lines) that is fixed in https://bugs.webkit.org/show_bug.cgi?id=229629.
This patch introduced a regression in Ubuntu 18.04. Compilation aborts on a linking error:
: && /usr/lib/ccache/g++-8 -fPIC -fdiagnostics-color=always -Wextra -Wall -Wno-expansion-to-defined -Wno-noexcept-type -Wno-psabi -Wno-misleading-indentation -Wno-maybe-uninitialized -Wwrite-strings -Wun
collect2: error: ld returned 1 exit status
[207/6083] Building CXX object Source/ThirdParty/ANGLE/CMakeFiles/ANGLE.dir/src/compiler/translator/tree_ops/RewriteAtomicFunctionExpressions.cpp.o
ninja: build stopped: subcommand failed.
program finished with exit code 254
This Ubuntu 18.04 bot uses GCC-8 (v8.4.0) and links with GNU Gold (GNU Binutils for Ubuntu 2.30 - 1.15).
On the Debian bot this regression didn't happen. It uses a similar compiler and linker. Compiler: v8.3.0. Linker: GNU Binutils for Debian 2.31.1 - 1.16.
Any clue or hint on what might have gone wrong?
There is a Debian bot with
(In reply to Diego Pino from comment #10)
> This patch introduced a regression in Ubuntu 18.04. Compilation aborts on a
> linking error:
> /usr/bin/ld.gold: error:
> DerivedSources/unified-sources/UnifiedSource-cd2e8cfa-2.cpp.o: requires
> dynamic R_X86_64_PC32 reloc again
> collect2: error: ld returned 1 exit status
> Any clue or hint on what might have gone wrong?
> There is a Debian bot with
Looks like your linker doesn't support linking the direct branch I used in the inline asm statement in ctiMasmProbeTrampoline. I can try re-instating the old way of working for non-darwin ports since the diff isn't big in for X86_64.
(In reply to Mark Lam from comment #11)
> Looks like your linker doesn't support linking the direct branch I used in
> the inline asm statement in ctiMasmProbeTrampoline. I can try re-instating
> the old way of working for non-darwin ports since the diff isn't big in for
FYI, also getting an ld fatal error with the Sun linker. It bails on the new X86_64 inline assembly instruction `call executeJSCJITProbe` (text relocation remains against symbol executeJSCJITProbe.)
If I change to `call executeJSCJITProbe@PLT`, it links and runs successfully. I am patching it locally like this:
@@ -596,7 +596,7 @@
"movq %xmm15, " STRINGIZE_VALUE_OF(PROBE_CPU_XMM15_OFFSET) "(%rsp)" "\n"
"movq %rsp, %rdi" "\n" // the Probe::State* arg.
- "call " SYMBOL_STRING(executeJSCJITProbe) "\n"
+ "call " GLOBAL_REFERENCE(executeJSCJITProbe) "\n"
// Make sure the Probe::State is entirely below the result stack pointer so
// that register values are still preserved when we call the initializeStack
where the GLOBAL_REFERENCE macro is appending @PLT.
I may be the same linker issue Diego is reporting in Ubuntu.
I applied Yusuke's speculative patch (r281911) instead of my own, and can confirm it clears the issue with the Sun linker.