Bug 175512

Summary: [ARM64] Use x29 and x30 instead of fp and lr to make GCC happy
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Csaba Osztrogonác <ossy>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, fpizlo, jfbastien, keith_miller, msaboff, ossy, rmorisset, saam, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch.
none
Patch
none
Patch for landing none

Description Mark Lam 2017-08-12 08:19:15 PDT
This is needed because Linux toolchains are not smart enough to resolve constexpr numeric expressions.  See https://bugs.webkit.org/show_bug.cgi?id=175446#c21.
Comment 1 Radar WebKit Bug Importer 2017-08-12 08:19:47 PDT
<rdar://problem/33863584>
Comment 2 Mark Lam 2017-08-12 08:23:49 PDT
Also need to add MacroAssemblerARM64.cpp to the JSC CMakeList.txt.
Comment 3 Mark Lam 2017-08-12 09:38:45 PDT
Created attachment 317997 [details]
proposed patch.
Comment 4 Mark Lam 2017-08-12 10:59:07 PDT
@Ossy, this patch works for OS(DARWIN) with Clang.  Can you please verify that this resolves the issue you reported in https://bugs.webkit.org/show_bug.cgi?id=175446#c21 ?
Comment 5 Csaba Osztrogonác 2017-08-12 13:26:35 PDT
(In reply to Mark Lam from comment #4)
> @Ossy, this patch works for OS(DARWIN) with Clang.  Can you please verify
> that this resolves the issue you reported in
> https://bugs.webkit.org/show_bug.cgi?id=175446#c21 ?

Thanks for the quick fix, I'll run a build and test soon.
Comment 6 Csaba Osztrogonác 2017-08-12 13:28:50 PDT
Now I got these errors:

{standard input}: Assembler messages:
{standard input}:34: Error: operand 2 should be an integer register -- `stp x28,fp,[sp,#240]'
{standard input}:37: Error: operand 1 should be an integer register -- `str lr,[sp,#552]'
{standard input}:38: Error: operand 1 should be an integer or stack pointer register -- `add lr,lr,#2*8'
{standard input}:39: Error: operand 1 should be an integer register -- `str lr,[sp,#272]'
{standard input}:94: Error: operand 1 should be an integer register -- `mov lr,#0'
{standard input}:104: Error: operand 1 should be an integer register -- `mov lr,#1'
{standard input}:106: Error: operand 1 should be an integer register -- `ldr fp,[sp,#552]'
{standard input}:114: Error: operand 1 should be an integer register -- `str lr,[sp,#16]'
{standard input}:115: Error: operand 2 should be an integer register -- `ldp x28,lr,[x27,#232]'
{standard input}:116: Error: operand 2 should be an integer register -- `stp x28,lr,[sp,#232]'
{standard input}:117: Error: operand 2 should be an integer register -- `ldp x28,lr,[x27,#248]'
{standard input}:118: Error: operand 2 should be an integer register -- `stp x28,lr,[sp,#248]'
{standard input}:119: Error: operand 2 should be an integer register -- `ldp x28,lr,[x27,#264]'
{standard input}:120: Error: operand 2 should be an integer register -- `stp x28,lr,[sp,#264]'
{standard input}:121: Error: operand 2 should be an integer register -- `ldp x28,lr,[x27,#280]'
{standard input}:122: Error: operand 2 should be an integer register -- `stp x28,lr,[sp,#280]'
{standard input}:123: Error: operand 1 should be an integer register -- `ldr lr,[sp,#16]'
{standard input}:125: Error: operand 1 should be an integer register -- `cbnz lr,.LctiMasmProbeTrampolineEnd'
{standard input}:126: Error: operand 1 should be an integer register -- `ldr lr,[sp,#264]'
{standard input}:127: Error: operand 1 should be an integer or stack pointer register -- `sub lr,lr,#(6*8)'
{standard input}:129: Error: integer 64-bit register expected at operand 2 -- `str x27,[lr,#(5*8)]'
{standard input}:130: Error: operand 1 should be an integer register -- `str lr,[sp,#264]'
{standard input}:131: Error: operand 1 should be an integer register -- `str fp,[sp,#272]'
{standard input}:133: Error: operand 1 should be an integer register -- `ldr lr,[sp,#264]'
{standard input}:134: Error: operand 1 should be an integer or stack pointer register -- `sub lr,lr,#(6*8)'
{standard input}:136: Error: integer 64-bit register expected at operand 3 -- `stp x27,x28,[lr,#(0*8)]'
{standard input}:138: Error: integer 64-bit register expected at operand 3 -- `stp x27,x28,[lr,#(2*8)]'
{standard input}:141: Error: integer 64-bit register expected at operand 3 -- `stp x27,x28,[lr,#(4*8)]'
{standard input}:147: Error: operand 1 should be a floating-point register -- `ldp fp,lr,[sp],#2*8'
Comment 7 Csaba Osztrogonác 2017-08-12 14:16:00 PDT
I checked it in details, it seems not the constexprs were the problem,
but fp and lr names. Replacing them with x29 and x30 made GCC happy.
I'll run tests and will upload a fix if everything is OK.
Comment 8 Csaba Osztrogonác 2017-08-12 15:32:32 PDT
Created attachment 318001 [details]
Patch
Comment 9 Mark Lam 2017-08-12 17:06:59 PDT
Comment on attachment 318001 [details]
Patch

r=me.  Please put a comment in ctiMasmProbeTrampoline to explain that why we're using x29 and x30, something like: "We use x29 and x30 instead of fp and lr because gcc's inline assembler does not recognize fp and lr."  This will minimize the chance that someone will change them back to sp and lr at a later date unknowingly.
Comment 10 Csaba Osztrogonác 2017-08-12 23:04:26 PDT
Created attachment 318005 [details]
Patch for landing
Comment 11 WebKit Commit Bot 2017-08-12 23:47:52 PDT
Comment on attachment 318005 [details]
Patch for landing

Clearing flags on attachment: 318005

Committed r220630: <http://trac.webkit.org/changeset/220630>
Comment 12 WebKit Commit Bot 2017-08-12 23:47:54 PDT
All reviewed patches have been landed.  Closing bug.