108261 2013-01-29 17:06:16 -0800 offlineasm BaseIndex handling is broken on ARM due to MIPS changes 2013-02-04 02:12:26 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 fpizlo kbalazs barraclough ddkilzer fu gergely ggaren kbalazs kilvadyb mark.lam mhahnenberg msaboff oliver psolanki sam webkit.review.bot oldest_to_newest 819528 0 fpizlo 2013-01-29 17:06:16 -0800 The riscLowerMalformedAddresses code is intended to only lower BaseIndex if the target architecture can't handle that specific form of BaseIndex. And when it does lower BaseIndex, it's supposed to lower it to a leap instruction, in recognition of the fact that the target architecture almost certainly has a more efficient way of handling add and shift in a single instruction. Currently the mips.rb backend overrides riscLowerMalformedAddressesRecurse for BaseIndex, effectively deactivating that optimization in its entirety. I will disable the overriding for now, which will bring ARM back to its proper glory. I'll leave it to the MIPS port to sort this out. Bottom line: you shouldn't be overriding methods in mips.rb, since mips.rb is included in *all* ports. This is by design since offlineasm is meant to be able to work with fat binaries, where a single invocation simultaneously generates code for multiple architectures. 819530 1 185351 fpizlo 2013-01-29 17:07:19 -0800 Created attachment 185351 the patch 819554 2 fpizlo 2013-01-29 17:20:19 -0800 MIPS code disabled in http://trac.webkit.org/changeset/141189 819556 3 185351 fpizlo 2013-01-29 17:20:38 -0800 Comment on attachment 185351 the patch Clearing flags since this landed in http://trac.webkit.org/changeset/141189 821349 4 kilvadyb 2013-01-31 03:59:40 -0800 (In reply to comment #0) > The riscLowerMalformedAddresses code is intended to only lower BaseIndex if the target architecture can't handle that specific form of BaseIndex. And when it does lower BaseIndex, it's supposed to lower it to a leap instruction, in recognition of the fact that the target architecture almost certainly has a more efficient way of handling add and shift in a single instruction. > > Currently the mips.rb backend overrides riscLowerMalformedAddressesRecurse for BaseIndex, effectively deactivating that optimization in its entirety. > > I will disable the overriding for now, which will bring ARM back to its proper glory. I'll leave it to the MIPS port to sort this out. > > Bottom line: you shouldn't be overriding methods in mips.rb, since mips.rb is included in *all* ports. This is by design since offlineasm is meant to be able to work with fat binaries, where a single invocation simultaneously generates code for multiple architectures. Thanks for the explanation, I am working on a proper solution. 821815 5 185835 kilvadyb 2013-01-31 12:16:26 -0800 Created attachment 185835 proposed patch. 821830 6 185835 fpizlo 2013-01-31 12:28:52 -0800 Comment on attachment 185835 proposed patch. I like this. r=me. 823230 7 185835 mark.lam 2013-02-01 13:51:23 -0800 Comment on attachment 185835 proposed patch. Since Filip r+'d this already and the cq? was added after, I'll cq+ it to move it along. 823241 8 185835 webkit.review.bot 2013-02-01 14:01:34 -0800 Comment on attachment 185835 proposed patch. Clearing flags on attachment: 185835 Committed r141641: <http://trac.webkit.org/changeset/141641> 823242 9 webkit.review.bot 2013-02-01 14:01:38 -0800 All reviewed patches have been landed. Closing bug. 185351 2013-01-29 17:07:19 -0800 2013-01-29 17:20:37 -0800 the patch blah.patch text/plain 1262 fpizlo SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTQxMTg2KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE0IEBA CisyMDEzLTAxLTI5ICBGaWxpcCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CisKKyAgICAgICAg b2ZmbGluZWFzbSBCYXNlSW5kZXggaGFuZGxpbmcgaXMgYnJva2VuIG9uIEFSTSBkdWUgdG8gTUlQ UyBjaGFuZ2VzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD0xMDgyNjEKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKyAgICAgICAg CisgICAgICAgIEJhY2tlbmRzIHNob3VsZG4ndCBvdmVycmlkZSBlYWNoIG90aGVyJ3MgbWV0aG9k cy4gVGhhdCdzIG5vdCBjb29sLgorCisgICAgICAgICogb2ZmbGluZWFzbS9taXBzLnJiOgorCiAy MDEzLTAxLTI5ICBGaWxpcCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CiAKICAgICAgICAgY2xv b3AucmIgc2hvdWxkbid0IHVzZSBhIG1ldGhvZCBjYWxsZWQgJ2R1bXAnIGZvciBjb2RlIGdlbmVy YXRpb24KSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9vZmZsaW5lYXNtL21pcHMucmIKPT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL29mZmxpbmVhc20vbWlwcy5yYgkocmV2 aXNpb24gMTQxMDc4KQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL29mZmxpbmVhc20vbWlwcy5y Ygkod29ya2luZyBjb3B5KQpAQCAtMzEzLDcgKzMxMyw3IEBAIGVuZAogIwogCiBjbGFzcyBCYXNl SW5kZXgKLSAgICBkZWYgcmlzY0xvd2VyTWFsZm9ybWVkQWRkcmVzc2VzUmVjdXJzZShsaXN0LCBu b2RlLCAmYmxvY2spCisgICAgZGVmIEJST0tFTl9yaXNjTG93ZXJNYWxmb3JtZWRBZGRyZXNzZXNS ZWN1cnNlKGxpc3QsIG5vZGUsICZibG9jaykKICAgICAgICAgaWYgc2NhbGVTaGlmdCA9PSAwCiAg ICAgICAgICAgICB0bXAwID0gVG1wLm5ldyhjb2RlT3JpZ2luLCA6Z3ByKQogICAgICAgICAgICAg bGlzdCA8PCBJbnN0cnVjdGlvbi5uZXcoY29kZU9yaWdpbiwgImFkZHAiLCBbYmFzZSwgaW5kZXgs IHRtcDBdKQo= 185835 2013-01-31 12:16:26 -0800 2013-02-01 14:01:33 -0800 proposed patch. llint.diff text/plain 3420 kilvadyb ZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvSmF2 YVNjcmlwdENvcmUvQ2hhbmdlTG9nCmluZGV4IDZmOWYwN2EuLjJhZDhlNWIgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL0phdmFTY3JpcHRD b3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDEzLTAxLTMxICBCYWxhenMgS2lsdmFk eSAgPGtpbHZhZHliQGhvbWVqaW5uaS5jb20+CisKKyAgICAgICAgb2ZmbGluZWFzbSBCYXNlSW5k ZXggaGFuZGxpbmcgaXMgYnJva2VuIG9uIEFSTSBkdWUgdG8gTUlQUyBjaGFuZ2VzCisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMDgyNjEKKworICAgICAg ICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBvZmZsaW5lYXNtIEJhc2VJ bmRleCBoYW5kbGluZyBmaXggb24gTUlQUy4KKworICAgICAgICAqIG9mZmxpbmVhc20vbWlwcy5y YjoKKyAgICAgICAgKiBvZmZsaW5lYXNtL3Jpc2MucmI6CisKIDIwMTMtMDEtMzEgIE1hcmsgSGFo bmVuYmVyZyAgPG1oYWhuZW5iZXJnQGFwcGxlLmNvbT4KIAogICAgICAgICBPYmplY3RpdmUtQyBB UEk6IEZpeCBpbnNlcnRpb24gb2YgdmFsdWVzIGdyZWF0ZXIgdGhhbiB0aGUgbWF4IGluZGV4IGFs bG93ZWQgYnkgdGhlIHNwZWMKZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9vZmZs aW5lYXNtL21pcHMucmIgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvb2ZmbGluZWFzbS9taXBzLnJi CmluZGV4IDNiNTBkNTEuLjNjYjVjZTggMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29y ZS9vZmZsaW5lYXNtL21pcHMucmIKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL29mZmxpbmVh c20vbWlwcy5yYgpAQCAtMzEyLDggKzMxMiwyMyBAQCBlbmQKICMgU3BlY2lhbGl6YXRpb24gb2Yg bG93ZXJpbmcgb2YgbWFsZm9ybWVkIEJhc2VJbmRleCBhZGRyZXNzZXMuCiAjCiAKK2NsYXNzIE5v ZGUKKyAgICBkZWYgbWlwc0xvd2VyTWFsZm9ybWVkQWRkcmVzc2VzUmVjdXJzZShsaXN0LCB0b3BM ZXZlbE5vZGUsICZibG9jaykKKyAgICAgICAgbWFwQ2hpbGRyZW4geworICAgICAgICAgICAgfCBz dWJOb2RlIHwKKyAgICAgICAgICAgIHN1Yk5vZGUubWlwc0xvd2VyTWFsZm9ybWVkQWRkcmVzc2Vz UmVjdXJzZShsaXN0LCB0b3BMZXZlbE5vZGUsICZibG9jaykKKyAgICAgICAgfQorICAgIGVuZAor ZW5kCisKK2NsYXNzIEFkZHJlc3MKKyAgICBkZWYgbWlwc0xvd2VyTWFsZm9ybWVkQWRkcmVzc2Vz UmVjdXJzZShsaXN0LCBub2RlLCAmYmxvY2spCisgICAgICAgIHJpc2NMb3dlck1hbGZvcm1lZEFk ZHJlc3Nlc1JlY3Vyc2UobGlzdCwgbm9kZSwgJmJsb2NrKQorICAgIGVuZAorZW5kCisKIGNsYXNz IEJhc2VJbmRleAotICAgIGRlZiBCUk9LRU5fcmlzY0xvd2VyTWFsZm9ybWVkQWRkcmVzc2VzUmVj dXJzZShsaXN0LCBub2RlLCAmYmxvY2spCisgICAgZGVmIG1pcHNMb3dlck1hbGZvcm1lZEFkZHJl c3Nlc1JlY3Vyc2UobGlzdCwgbm9kZSwgJmJsb2NrKQogICAgICAgICBpZiBzY2FsZVNoaWZ0ID09 IDAKICAgICAgICAgICAgIHRtcDAgPSBUbXAubmV3KGNvZGVPcmlnaW4sIDpncHIpCiAgICAgICAg ICAgICBsaXN0IDw8IEluc3RydWN0aW9uLm5ldyhjb2RlT3JpZ2luLCAiYWRkcCIsIFtiYXNlLCBp bmRleCwgdG1wMF0pCkBAIC0zMjcsNiArMzQyLDIxIEBAIGNsYXNzIEJhc2VJbmRleAogICAgIGVu ZAogZW5kCiAKK2NsYXNzIEFic29sdXRlQWRkcmVzcworICAgIGRlZiBtaXBzTG93ZXJNYWxmb3Jt ZWRBZGRyZXNzZXNSZWN1cnNlKGxpc3QsIG5vZGUsICZibG9jaykKKyAgICAgICAgcmlzY0xvd2Vy TWFsZm9ybWVkQWRkcmVzc2VzUmVjdXJzZShsaXN0LCBub2RlLCAmYmxvY2spCisgICAgZW5kCitl bmQKKworZGVmIG1pcHNMb3dlck1hbGZvcm1lZEFkZHJlc3NlcyhsaXN0LCAmYmxvY2spCisgICAg bmV3TGlzdCA9IFtdCisgICAgbGlzdC5lYWNoIHsKKyAgICAgICAgfCBub2RlIHwKKyAgICAgICAg bmV3TGlzdCA8PCBub2RlLm1pcHNMb3dlck1hbGZvcm1lZEFkZHJlc3Nlc1JlY3Vyc2UobmV3TGlz dCwgbm9kZSwgJmJsb2NrKQorICAgIH0KKyAgICBuZXdMaXN0CitlbmQKKwogIwogIyBMb3dlcmlu ZyBvZiBtaXNwbGFjZWQgaW1tZWRpYXRlcyBvZiBNSVBTIHNwZWNpZmljIGluc3RydWN0aW9ucy4g Rm9yIGV4YW1wbGU6CiAjCkBAIC01MzIsNyArNTYyLDcgQEAgY2xhc3MgU2VxdWVuY2UKICAgICAg ICAgcmVzdWx0ID0gcmlzY0xvd2VyU2ltcGxlQnJhbmNoT3BzKHJlc3VsdCkKICAgICAgICAgcmVz dWx0ID0gcmlzY0xvd2VySGFyZEJyYW5jaE9wcyhyZXN1bHQpCiAgICAgICAgIHJlc3VsdCA9IHJp c2NMb3dlclNoaWZ0T3BzKHJlc3VsdCkKLSAgICAgICAgcmVzdWx0ID0gcmlzY0xvd2VyTWFsZm9y bWVkQWRkcmVzc2VzKHJlc3VsdCkgeworICAgICAgICByZXN1bHQgPSBtaXBzTG93ZXJNYWxmb3Jt ZWRBZGRyZXNzZXMocmVzdWx0KSB7CiAgICAgICAgICAgICB8IG5vZGUsIGFkZHJlc3MgfAogICAg ICAgICAgICAgaWYgYWRkcmVzcy5pc19hPyBBZGRyZXNzCiAgICAgICAgICAgICAgICAgKC0weGZm ZmYuLjB4ZmZmZikuaW5jbHVkZT8gYWRkcmVzcy5vZmZzZXQudmFsdWUKZGlmZiAtLWdpdCBhL1Nv dXJjZS9KYXZhU2NyaXB0Q29yZS9vZmZsaW5lYXNtL3Jpc2MucmIgYi9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvb2ZmbGluZWFzbS9yaXNjLnJiCmluZGV4IDQ0YjRkYmQuLjc0MDgyNTMgMTAwNjQ0Ci0t LSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9vZmZsaW5lYXNtL3Jpc2MucmIKKysrIGIvU291cmNl L0phdmFTY3JpcHRDb3JlL29mZmxpbmVhc20vcmlzYy5yYgpAQCAtMTg3LDcgKzE4Nyw3IEBAIGNs YXNzIE5vZGUKIGVuZAogCiBjbGFzcyBBZGRyZXNzCi0gICAgZGVmIHJpc2NMb3dlck1hbGZvcm1l ZEFkZHJlc3Nlc1JlY3Vyc2UobGlzdCwgbm9kZSkKKyAgICBkZWYgcmlzY0xvd2VyTWFsZm9ybWVk QWRkcmVzc2VzUmVjdXJzZShsaXN0LCBub2RlLCAmYmxvY2spCiAgICAgICAgIHJldHVybiBzZWxm IGlmIHlpZWxkIG5vZGUsIHNlbGYKIAogICAgICAgICB0bXAgPSBUbXAubmV3KGNvZGVPcmlnaW4s IDpncHIpCkBAIC0yMDgsNyArMjA4LDcgQEAgY2xhc3MgQmFzZUluZGV4CiBlbmQKIAogY2xhc3Mg QWJzb2x1dGVBZGRyZXNzCi0gICAgZGVmIHJpc2NMb3dlck1hbGZvcm1lZEFkZHJlc3Nlc1JlY3Vy c2UobGlzdCwgbm9kZSkKKyAgICBkZWYgcmlzY0xvd2VyTWFsZm9ybWVkQWRkcmVzc2VzUmVjdXJz ZShsaXN0LCBub2RlLCAmYmxvY2spCiAgICAgICAgIHJldHVybiBzZWxmIGlmIHlpZWxkIG5vZGUs IHNlbGYKICAgICAgICAgCiAgICAgICAgIHRtcCA9IFRtcC5uZXcoY29kZU9yaWdpbiwgOmdwcikK