173922 2017-06-28 07:02:20 -0700 Fix the AArch64 build after r218869 2017-06-28 10:23:50 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED DUPLICATE 173700 https://bugs.webkit.org/show_bug.cgi?id=173929 P1 Blocker --- 108645 1 ossy jfbastien clopez fpizlo jfbastien mark.lam saam oldest_to_newest 1323522 0 ossy 2017-06-28 07:02:20 -0700 https://trac.webkit.org/changeset/218869 (bug173700) broke the AArch64 build: https://build.webkit.org/builders/JSCOnly%20Linux%20AArch64%20Release/builds/1126 {standard input}: Assembler messages: {standard input}:28: Error: unexpected register in the immediate operand at operand 3 -- `subs xzr,x3,sp' {standard input}:120: Error: unexpected register in the immediate operand at operand 3 -- `subs xzr,x3,sp' note: I just noticed this build failure, no time and plan to fix it myself. 1323537 1 jfbastien 2017-06-28 08:47:37 -0700 I'll take a look. 1323545 2 jfbastien 2017-06-28 09:10:04 -0700 Yeah on ARM64 that second register can't be SP, because register 31 means ZR here. 1323549 3 jfbastien 2017-06-28 09:21:43 -0700 I think we want this: diff --git a/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm b/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm index fd35381d..0fb0254 100644 --- a/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm +++ b/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm @@ -136,7 +136,7 @@ macro doVMEntry(makeCall) addp CallFrameHeaderSlots, t4, t4 lshiftp 3, t4 subp sp, t4, t3 - bpa t3, sp, .throwStackOverflow + bibeq sp, t3, .throwStackOverflow # Ensure that we have enough additional stack capacity for the incoming args, # and the frame for the JS code we're executing. We need to do this check It still makes the assembler sad though. I think it expects "wsp" and is sad when it just sees "sp"? Looking... 1323558 4 jfbastien 2017-06-28 09:27:18 -0700 Ugh this is weird. arm64.rb:arm64Operand can't return "wsp" for "sp" all the time because other instructions get sad. But manually editing DerivedSources/JavaScriptCore/LLIntAssembly.h to "wsp" in those two spots works. So I can hack up arm64.rb:lowerARM64's handing for these branch instructions... 1323565 5 jfbastien 2017-06-28 09:35:18 -0700 Oh derp, the problem is we're using w registers instead of x. This should be 64-bit, not 32, and the "sp" is implicitly 64 so no need for "xsp". Why is are we doing that though? Looking more... 1323567 6 jfbastien 2017-06-28 09:39:08 -0700 OK bqbeq is the right thing, since it's quad. I now know a bit more llint. 1323568 7 314032 jfbastien 2017-06-28 09:42:06 -0700 Created attachment 314032 patch 1323571 8 jfbastien 2017-06-28 09:46:03 -0700 Another option is to teach all the branches to flip their operands and the condition if the last operand is SP. I'm not sure that's worth doing in this patch. 1323572 9 314032 mark.lam 2017-06-28 09:46:44 -0700 Comment on attachment 314032 patch this works as a workaround but bpa should also work because p stands for ointer and pointer on 64bit should be quad 1323573 10 jfbastien 2017-06-28 09:47:43 -0700 The patch got reverted, I'll re-apply it with this fix. 1323574 11 jfbastien 2017-06-28 09:51:19 -0700 Will fix in 173700. *** This bug has been marked as a duplicate of bug 173700 *** 314032 2017-06-28 09:42:06 -0700 2017-06-28 09:51:31 -0700 patch 0001-Fix-the-AArch64-build-after-r218869.patch text/plain 1858 jfbastien RnJvbSBlYzcxZDEyYTlhZTEzMDJkMmUwYmFmNTRhNzE1YjcxZDdkYjZhNTMwIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBKRiBCYXN0aWVuIDxqZmJhc3RpZW5AYXBwbGUuY29tPgpEYXRl OiBXZWQsIDI4IEp1biAyMDE3IDA5OjQxOjM0IC0wNzAwClN1YmplY3Q6IFtQQVRDSF0gRml4IHRo ZSBBQXJjaDY0IGJ1aWxkIGFmdGVyIHIyMTg4NjkKCi0tLQogU291cmNlL0phdmFTY3JpcHRDb3Jl L0NoYW5nZUxvZyAgICAgICAgICAgICAgICAgICAgICAgfCAxNCArKysrKysrKysrKysrKwogU291 cmNlL0phdmFTY3JpcHRDb3JlL2xsaW50L0xvd0xldmVsSW50ZXJwcmV0ZXI2NC5hc20gfCAgMiAr LQogMiBmaWxlcyBjaGFuZ2VkLCAxNSBpbnNlcnRpb25zKCspLCAxIGRlbGV0aW9uKC0pCgpkaWZm IC0tZ2l0IGEvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9KYXZhU2Ny aXB0Q29yZS9DaGFuZ2VMb2cKaW5kZXggNTRlZjBhNC4uMGQ1ZmNiYiAxMDA2NDQKLS0tIGEvU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUv Q2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTctMDYtMjggIEpGIEJhc3RpZW4gIDxqZmJh c3RpZW5AYXBwbGUuY29tPgorCisgICAgICAgIEZpeCB0aGUgQUFyY2g2NCBidWlsZCBhZnRlciBy MjE4ODY5CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0x NzM5MjIKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBU aGUgbGxpbnQgY29kZSB1c2VkIGdlbmVyYXRlZCBgc3VicyB4enIsIHgzLCBzcGAgd2hpY2ggaXNu J3QgdmFsaWQKKyAgICAgICAgQVJNNjQ6IHRoZSB0aGlyZCBvcGVyYW5kIGNhbm5vdCBiZSBTUCAo dGhhdCBlbmNvZGluZyB3b3VsZCBiZSBaUgorICAgICAgICBpbnN0ZWFkLCBzdWJ0cmFjdGluZyB6 ZXJvKS4gRmxpcCB0aGUgY29tcGFyaXNvbiBhbmQgb3BlcmFuZHMgdG8KKyAgICAgICAgZW1pdCB2 YWxpZCBjb2RlIChiZWNhdXNlIHRoZSBzZWNvbmQgb3BlcmFuZCBjYW4gYmUgU1ApLgorCisgICAg ICAgICogbGxpbnQvTG93TGV2ZWxJbnRlcnByZXRlcjY0LmFzbToKKwogMjAxNy0wNi0yOCAgTWFy ayBMYW0gIDxtYXJrLmxhbUBhcHBsZS5jb20+CiAKICAgICAgICAgR2FyZGVuaW5nOiBDTG9vcCBi dWlsZCBmaXguCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvbGxpbnQvTG93TGV2 ZWxJbnRlcnByZXRlcjY0LmFzbSBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9sbGludC9Mb3dMZXZl bEludGVycHJldGVyNjQuYXNtCmluZGV4IGZkMzUzODFkLi5jODQ5MTA5IDEwMDY0NAotLS0gYS9T b3VyY2UvSmF2YVNjcmlwdENvcmUvbGxpbnQvTG93TGV2ZWxJbnRlcnByZXRlcjY0LmFzbQorKysg Yi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvbGxpbnQvTG93TGV2ZWxJbnRlcnByZXRlcjY0LmFzbQpA QCAtMTM2LDcgKzEzNiw3IEBAIG1hY3JvIGRvVk1FbnRyeShtYWtlQ2FsbCkKICAgICBhZGRwIENh bGxGcmFtZUhlYWRlclNsb3RzLCB0NCwgdDQKICAgICBsc2hpZnRwIDMsIHQ0CiAgICAgc3VicCBz cCwgdDQsIHQzCi0gICAgYnBhIHQzLCBzcCwgLnRocm93U3RhY2tPdmVyZmxvdworICAgIGJxYmVx IHNwLCB0MywgLnRocm93U3RhY2tPdmVyZmxvdwogCiAgICAgIyBFbnN1cmUgdGhhdCB3ZSBoYXZl IGVub3VnaCBhZGRpdGlvbmFsIHN0YWNrIGNhcGFjaXR5IGZvciB0aGUgaW5jb21pbmcgYXJncywK ICAgICAjIGFuZCB0aGUgZnJhbWUgZm9yIHRoZSBKUyBjb2RlIHdlJ3JlIGV4ZWN1dGluZy4gV2Ug bmVlZCB0byBkbyB0aGlzIGNoZWNrCi0tIAoyLjEwLjEKCg==