141548 2015-02-12 19:51:32 -0800 Generate incq instead of addq when the immediate value is one 2015-02-13 13:54:46 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 benjamin benjamin barraclough msaboff oliver oldest_to_newest 1068825 0 benjamin 2015-02-12 19:51:32 -0800 Generate incq instead of addq when the immediate value is one 1068828 1 246499 benjamin 2015-02-12 19:56:29 -0800 Created attachment 246499 Patch 1068835 2 246499 barraclough 2015-02-12 21:00:45 -0800 Comment on attachment 246499 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=246499&action=review Have you performance tested? – inc used to be slower on some old intel chips (since it only sets 5/6 of the main flags) so add was often preferable – though that may well no longer be the case. > Source/JavaScriptCore/assembler/X86Assembler.h:566 > + m_formatter.oneByteOp64(OP_GROUP5_Ev, GROUP1_OP_ADD, base, offset); Shouldn't really be using 'GROUP1_OP_ADD' here – I think this should use 'GROUP5_OP_INC' (defining it, if it doesn't exist). 1068864 3 benjamin 2015-02-13 00:06:51 -0800 (In reply to comment #2) > Comment on attachment 246499 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=246499&action=review > > Have you performance tested? – inc used to be slower on some old intel chips > (since it only sets 5/6 of the main flags) so add was often preferable – > though that may well no longer be the case. I was intrigued by your comment so I looked into the perf tables all the way to pentium. For throughput and latency, Inc was equal or better than Add in all the cases I looked at. I checked the Intel Perf guideline, it says: "INC and DEC instructions should be replaced with ADD or SUB instructions, because ADD and SUB overwrite all flags, whereas INC and DEC do not, therefore creating false dependencies on earlier instructions that set the flags." -> exactly what you were saying. I checked what other compilers do. GCC seems to use ADD systematically. Clang uses INC systematically. ICC uses INC systematically. I'll have to test on our benchmarks... 1069021 4 benjamin 2015-02-13 13:54:46 -0800 Committed r180075: <http://trac.webkit.org/changeset/180075> 246499 2015-02-12 19:56:29 -0800 2015-02-12 21:00:45 -0800 Patch bug-141548-20150212195627.patch text/plain 2396 benjamin U3VidmVyc2lvbiBSZXZpc2lvbjogMTc5OTU3CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCAz MzUxODlmNTQ0MmM3NzI4ODQwODA3ZWI5OWQwYmE5YzIzMzM2ZmRjLi4zYjFiMjYxNmFiYWU0N2Ni ZDczMjQ0N2ExZWJkYzhlNWM1NjlhZjA0IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs NSArMSwyMSBAQAogMjAxNS0wMi0xMiAgQmVuamFtaW4gUG91bGFpbiAgPGJwb3VsYWluQGFwcGxl LmNvbT4KIAorICAgICAgICBHZW5lcmF0ZSBpbmNxIGluc3RlYWQgb2YgYWRkcSB3aGVuIHRoZSBp bW1lZGlhdGUgdmFsdWUgaXMgb25lCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3No b3dfYnVnLmNnaT9pZD0xNDE1NDgKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICBKU0MgZW1pdHMgImFkZHEgIzEgKHJYWCkiICphIGxvdCouCisgICAgICAg IFRoaXMgcGF0Y2ggcmVwbGFjZSB0aGF0IGJ5IGluY3EsIHdoaWNoIGlzIG9uZSBieXRlIHNob3J0 ZXIKKyAgICAgICAgYW5kIGlzIHRoZSBhZHZpY2VkIGZvcm0uCisKKyAgICAgICAgKiBhc3NlbWJs ZXIvTWFjcm9Bc3NlbWJsZXJYODZfNjQuaDoKKyAgICAgICAgKEpTQzo6TWFjcm9Bc3NlbWJsZXJY ODZfNjQ6OmFkZDY0KToKKyAgICAgICAgKiBhc3NlbWJsZXIvWDg2QXNzZW1ibGVyLmg6CisgICAg ICAgIChKU0M6Olg4NkFzc2VtYmxlcjo6aW5jcV9tKToKKworMjAxNS0wMi0xMiAgQmVuamFtaW4g UG91bGFpbiAgPGJwb3VsYWluQGFwcGxlLmNvbT4KKwogICAgICAgICBBcml0aFNxcnQgc2hvdWxk IG5vdCBiZSBjb25kaXRpb25hbCBvbiBzdXBwb3J0c0Zsb2F0aW5nUG9pbnRTcXJ0CiAgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNDE1NDYKIApkaWZmIC0t Z2l0IGEvU291cmNlL0phdmFTY3JpcHRDb3JlL2Fzc2VtYmxlci9NYWNyb0Fzc2VtYmxlclg4Nl82 NC5oIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL2Fzc2VtYmxlci9NYWNyb0Fzc2VtYmxlclg4Nl82 NC5oCmluZGV4IDlkYTk4Y2UyNDg0YzI2ODI0ZTcwYTI5NTUyYTQyZjNjZDNiMTEzYmEuLmE4MjQz ZTJjZjU0MzhlNjU4MGRlZmE0MzUyZTEzY2M0MTdiNWUzNzEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9K YXZhU2NyaXB0Q29yZS9hc3NlbWJsZXIvTWFjcm9Bc3NlbWJsZXJYODZfNjQuaAorKysgYi9Tb3Vy Y2UvSmF2YVNjcmlwdENvcmUvYXNzZW1ibGVyL01hY3JvQXNzZW1ibGVyWDg2XzY0LmgKQEAgLTI5 MSw3ICsyOTEsMTAgQEAgcHVibGljOgogCiAgICAgdm9pZCBhZGQ2NChUcnVzdGVkSW1tMzIgaW1t LCBBZGRyZXNzIGFkZHJlc3MpCiAgICAgewotICAgICAgICBtX2Fzc2VtYmxlci5hZGRxX2ltKGlt bS5tX3ZhbHVlLCBhZGRyZXNzLm9mZnNldCwgYWRkcmVzcy5iYXNlKTsKKyAgICAgICAgaWYgKGlt bS5tX3ZhbHVlID09IDEpCisgICAgICAgICAgICBtX2Fzc2VtYmxlci5pbmNxX20oYWRkcmVzcy5v ZmZzZXQsIGFkZHJlc3MuYmFzZSk7CisgICAgICAgIGVsc2UKKyAgICAgICAgICAgIG1fYXNzZW1i bGVyLmFkZHFfaW0oaW1tLm1fdmFsdWUsIGFkZHJlc3Mub2Zmc2V0LCBhZGRyZXNzLmJhc2UpOwog ICAgIH0KIAogICAgIHZvaWQgYWRkNjQoVHJ1c3RlZEltbTMyIGltbSwgQWJzb2x1dGVBZGRyZXNz IGFkZHJlc3MpCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvYXNzZW1ibGVyL1g4 NkFzc2VtYmxlci5oIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL2Fzc2VtYmxlci9YODZBc3NlbWJs ZXIuaAppbmRleCAyYzhkODY3YzMxNTY3ZDQ4Y2MyNmNmNGMzNDVhY2JiMTUwM2ZlY2IyLi43ODc3 ZWI3MGRhYzgxNjIzYzRmMTU0NDAyOTA4MWMxNzU1YTNjZWEzIDEwMDY0NAotLS0gYS9Tb3VyY2Uv SmF2YVNjcmlwdENvcmUvYXNzZW1ibGVyL1g4NkFzc2VtYmxlci5oCisrKyBiL1NvdXJjZS9KYXZh U2NyaXB0Q29yZS9hc3NlbWJsZXIvWDg2QXNzZW1ibGVyLmgKQEAgLTU2MCw2ICs1NjAsMTEgQEAg cHVibGljOgogICAgIHsKICAgICAgICAgbV9mb3JtYXR0ZXIub25lQnl0ZU9wNjQoT1BfR1JPVVA1 X0V2LCBHUk9VUDFfT1BfQURELCBkc3QpOwogICAgIH0KKworICAgIHZvaWQgaW5jcV9tKGludCBv ZmZzZXQsIFJlZ2lzdGVySUQgYmFzZSkKKyAgICB7CisgICAgICAgIG1fZm9ybWF0dGVyLm9uZUJ5 dGVPcDY0KE9QX0dST1VQNV9FdiwgR1JPVVAxX09QX0FERCwgYmFzZSwgb2Zmc2V0KTsKKyAgICB9 CiAjZW5kaWYgLy8gQ1BVKFg4Nl82NCkKIAogICAgIHZvaWQgbmVnbF9yKFJlZ2lzdGVySUQgZHN0 KQo=