Bug 175512 - [ARM64] Use x29 and x30 instead of fp and lr to make GCC happy
Summary: [ARM64] Use x29 and x30 instead of fp and lr to make GCC happy
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Csaba Osztrogonác
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-08-12 08:19 PDT by Mark Lam
Modified: 2017-08-12 23:47 PDT (History)
10 users (show)

See Also:


Attachments
proposed patch. (13.55 KB, patch)
2017-08-12 09:38 PDT, Mark Lam
no flags Details | Formatted Diff | Diff
Patch (9.98 KB, patch)
2017-08-12 15:32 PDT, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff
Patch for landing (10.44 KB, patch)
2017-08-12 23:04 PDT, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.