106729 2013-01-12 03:02:41 -0800 Use __sync_add_and_fetch instead of __gnu_cxx::__exchange_and_add 2013-01-13 01:27:11 -0800 1 1 1 Unclassified WebKit Web Template Framework 528+ (Nightly build) All All RESOLVED FIXED P1 Blocker --- 106739 106740 106708 1 ossy ossy andersca ap bdakin beidson benjamin darin d-r enrica hausmann hyatt kling mitz mjs mrowe ojan.autocc ossy sam simon.fraser thorton tmpsantos webkit.review.bot zan oldest_to_newest 805856 0 ossy 2013-01-12 03:02:41 -0800 https://trac.webkit.org/changeset/139514 broke the GCC WK2 builds (EFL,Qt,GTK), because atomicIncrement implementation in WTF use __gnu_cxx::__exchange_and_add which supports only 32 bit int values. We should use __sync_add_and_fetch instead of __gnu_cxx::__exchange_and_add which supports all types. Fix is coming. 805857 1 182464 ossy 2013-01-12 03:04:43 -0800 Created attachment 182464 Patch 805858 2 ossy 2013-01-12 03:06:32 -0800 It is the only one blocker now to fix the Qt WK2 build on Linux. 805860 3 182464 ossy 2013-01-12 03:15:58 -0800 Comment on attachment 182464 Patch and add cq?, because I won't be online all the weekend to land it manually. 805873 4 zan 2013-01-12 05:56:51 -0800 (In reply to comment #2) > It is the only one blocker now to fix the Qt WK2 build on Linux. Same for the GTK port. 805888 5 182464 benjamin 2013-01-12 11:09:07 -0800 Comment on attachment 182464 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=182464&action=review > Source/WTF/ChangeLog:7 > + Use __sync_add_and_fetch instead of __gnu_cxx::__exchange_and_add > + https://bugs.webkit.org/show_bug.cgi?id=106729 > + > + Reviewed by NOBODY (OOPS!). > + You should have a bug description in your change log. > Source/WTF/ChangeLog:11 > + (WTF): You can remove this. > Source/WTF/wtf/Atomics.h:125 > +inline int atomicIncrement(int volatile* addend) { return __sync_add_and_fetch(addend, 1); } > +inline int atomicDecrement(int volatile* addend) { return __sync_sub_and_fetch(addend, 1); } > + > +inline int64_t atomicIncrement(int64_t volatile* addend) { return __sync_add_and_fetch(addend, 1); } > +inline int64_t atomicDecrement(int64_t volatile* addend) { return __sync_sub_and_fetch(addend, 1); } Is there any reason to keep this separated from Darwin with the change? 805889 6 182474 ossy 2013-01-12 11:34:12 -0800 Created attachment 182474 Patch 805890 7 ossy 2013-01-12 11:36:37 -0800 (In reply to comment #5) > (From update of attachment 182464 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=182464&action=review > > > Source/WTF/ChangeLog:7 > > + Use __sync_add_and_fetch instead of __gnu_cxx::__exchange_and_add > > + https://bugs.webkit.org/show_bug.cgi?id=106729 > > + > > + Reviewed by NOBODY (OOPS!). > > + > > You should have a bug description in your change log. Done. > > Source/WTF/ChangeLog:11 > > + (WTF): > > You can remove this. Done. > > Source/WTF/wtf/Atomics.h:125 > > +inline int atomicIncrement(int volatile* addend) { return __sync_add_and_fetch(addend, 1); } > > +inline int atomicDecrement(int volatile* addend) { return __sync_sub_and_fetch(addend, 1); } > > + > > +inline int64_t atomicIncrement(int64_t volatile* addend) { return __sync_add_and_fetch(addend, 1); } > > +inline int64_t atomicDecrement(int64_t volatile* addend) { return __sync_sub_and_fetch(addend, 1); } > > Is there any reason to keep this separated from Darwin with the change? I have no idea why Darwin implementation is different, it uses OSAtomicIncrement32Barrier and OSAtomicIncrement64Barrier. I don't know if Darwin has __sync_sub_and_fetch ... 805891 8 benjamin 2013-01-12 11:40:43 -0800 > > Is there any reason to keep this separated from Darwin with the change? > I have no idea why Darwin implementation is different, it uses OSAtomicIncrement32Barrier and OSAtomicIncrement64Barrier. I don't > know if Darwin has __sync_sub_and_fetch ... It is just a compiler builtin. I don't see why it would not be on Darwin. That's not a big concern, let's land this so Qt/GTK/EFL build again. 805895 9 182474 webkit.review.bot 2013-01-12 12:24:45 -0800 Comment on attachment 182474 Patch Clearing flags on attachment: 182474 Committed r139553: <http://trac.webkit.org/changeset/139553> 805896 10 webkit.review.bot 2013-01-12 12:24:50 -0800 All reviewed patches have been landed. Closing bug. 182464 2013-01-12 03:04:43 -0800 2013-01-12 11:34:06 -0800 Patch bug-106729-20130112030144.patch text/plain 1799 ossy U3VidmVyc2lvbiBSZXZpc2lvbjogMTM5NTQ2CmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL0NoYW5n ZUxvZyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCmluZGV4IDg1ODA5ZTlmMjk5ZWY4NDc0MzE0NjA3 MDBmZTUzOTA4MTI0MTJmOGQuLjgxZjJkNTViNGQ2ZTQ3MzA5ZTgyMjM5MDg0OTAzZjUxYWM5ZmVk MDcgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XVEYvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTMtMDEtMTIgIENzYWJhIE9zenRyb2dvbsOhYyAg PG9zc3lAd2Via2l0Lm9yZz4KKworICAgICAgICBVc2UgX19zeW5jX2FkZF9hbmRfZmV0Y2ggaW5z dGVhZCBvZiBfX2dudV9jeHg6Ol9fZXhjaGFuZ2VfYW5kX2FkZAorICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTA2NzI5CisKKyAgICAgICAgUmV2aWV3ZWQg YnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiB3dGYvQXRvbWljcy5oOgorICAgICAgICAo V1RGOjphdG9taWNJbmNyZW1lbnQpOgorICAgICAgICAoV1RGOjphdG9taWNEZWNyZW1lbnQpOgor ICAgICAgICAoV1RGKToKKwogMjAxMy0wMS0xMSAgRmlsaXAgUGl6bG8gIDxmcGl6bG9AYXBwbGUu Y29tPgogCiAgICAgICAgIEFkZCBXVEZfRVhQT1JUX1BSSVZBVEUgdG8gcHJpbnRJbnRlcm5hbCgp IG1ldGhvZHMgb2YgUHJpbnRTdHJlYW0uaApkaWZmIC0tZ2l0IGEvU291cmNlL1dURi93dGYvQXRv bWljcy5oIGIvU291cmNlL1dURi93dGYvQXRvbWljcy5oCmluZGV4IDc1MGUxMDkyYWQzN2E1N2Uy NWUyZmRiNzI5MmZmYmQ1YjQzOWM3MDguLjg2ZWQ4MDVhMWUzYWRlNmY5NDJkMzZjNDc5MzZlZjdj MDc1NGE0NTQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvd3RmL0F0b21pY3MuaAorKysgYi9Tb3Vy Y2UvV1RGL3d0Zi9BdG9taWNzLmgKQEAgLTExOCw4ICsxMTgsMTEgQEAgaW5saW5lIGludCBhdG9t aWNEZWNyZW1lbnQoaW50IHZvbGF0aWxlKiBhZGRlbmQpIHsgcmV0dXJuIF9fYXRvbWljX2RlYyhh ZGRlbmQpIC0KICNlbGlmIENPTVBJTEVSKEdDQykgJiYgIUNQVShTUEFSQzY0KSAvLyBzaXplb2Yo X0F0b21pY193b3JkKSAhPSBzaXplb2YoaW50KSBvbiBzcGFyYzY0IGdjYwogI2RlZmluZSBXVEZf VVNFX0xPQ0tGUkVFX1RIUkVBRFNBRkVSRUZDT1VOVEVEIDEKIAotaW5saW5lIGludCBhdG9taWNJ bmNyZW1lbnQoaW50IHZvbGF0aWxlKiBhZGRlbmQpIHsgcmV0dXJuIF9fZ251X2N4eDo6X19leGNo YW5nZV9hbmRfYWRkKGFkZGVuZCwgMSkgKyAxOyB9Ci1pbmxpbmUgaW50IGF0b21pY0RlY3JlbWVu dChpbnQgdm9sYXRpbGUqIGFkZGVuZCkgeyByZXR1cm4gX19nbnVfY3h4OjpfX2V4Y2hhbmdlX2Fu ZF9hZGQoYWRkZW5kLCAtMSkgLSAxOyB9CitpbmxpbmUgaW50IGF0b21pY0luY3JlbWVudChpbnQg dm9sYXRpbGUqIGFkZGVuZCkgeyByZXR1cm4gX19zeW5jX2FkZF9hbmRfZmV0Y2goYWRkZW5kLCAx KTsgfQoraW5saW5lIGludCBhdG9taWNEZWNyZW1lbnQoaW50IHZvbGF0aWxlKiBhZGRlbmQpIHsg cmV0dXJuIF9fc3luY19zdWJfYW5kX2ZldGNoKGFkZGVuZCwgMSk7IH0KKworaW5saW5lIGludDY0 X3QgYXRvbWljSW5jcmVtZW50KGludDY0X3Qgdm9sYXRpbGUqIGFkZGVuZCkgeyByZXR1cm4gX19z eW5jX2FkZF9hbmRfZmV0Y2goYWRkZW5kLCAxKTsgfQoraW5saW5lIGludDY0X3QgYXRvbWljRGVj cmVtZW50KGludDY0X3Qgdm9sYXRpbGUqIGFkZGVuZCkgeyByZXR1cm4gX19zeW5jX3N1Yl9hbmRf ZmV0Y2goYWRkZW5kLCAxKTsgfQogCiAjZW5kaWYKIAo= 182474 2013-01-12 11:34:12 -0800 2013-01-12 12:24:45 -0800 Patch bug-106729-20130112113113.patch text/plain 2045 ossy U3VidmVyc2lvbiBSZXZpc2lvbjogMTM5NTQ2CmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL0NoYW5n ZUxvZyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCmluZGV4IDg1ODA5ZTlmMjk5ZWY4NDc0MzE0NjA3 MDBmZTUzOTA4MTI0MTJmOGQuLmM0YzBjOTdiMTFmZWFhNTI5MDllZTcxOGIyOTA1ODZhMTJkMTJi ZGEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XVEYvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMTggQEAKKzIwMTMtMDEtMTIgIENzYWJhIE9zenRyb2dvbsOhYyAg PG9zc3lAd2Via2l0Lm9yZz4KKworICAgICAgICBVc2UgX19zeW5jX2FkZF9hbmRfZmV0Y2ggaW5z dGVhZCBvZiBfX2dudV9jeHg6Ol9fZXhjaGFuZ2VfYW5kX2FkZAorICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTA2NzI5CisKKyAgICAgICAgQWZ0ZXIgcjEz OTUxNCB3ZSBuZWVkIGF0b21pY0luY3JlbWVudChpbnQ2NF90IHZvbGF0aWxlKikgZm9yIGFsbCBw bGF0Zm9ybS4gTm93IHRoZQorICAgICAgICBHQ0MgaW1wbGVtZW50YXRpb24gb2YgYXRvbWljSW5j cmVtZW50KCkgaXMgYmFzZWQgb24gX19nbnVfY3h4OjpfX2V4Y2hhbmdlX2FuZF9hZGQsCisgICAg ICAgIHdoaWNoIGRvZXNuJ3Qgc3VwcG9ydCBpbnQ2NF90IHR5cGUsIGJ1dCBfX3N5bmNfYWRkX2Fu ZF9mZXRjaCBkb2VzLgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgICogd3RmL0F0b21pY3MuaDoKKyAgICAgICAgKFdURjo6YXRvbWljSW5jcmVtZW50KToK KyAgICAgICAgKFdURjo6YXRvbWljRGVjcmVtZW50KToKKwogMjAxMy0wMS0xMSAgRmlsaXAgUGl6 bG8gIDxmcGl6bG9AYXBwbGUuY29tPgogCiAgICAgICAgIEFkZCBXVEZfRVhQT1JUX1BSSVZBVEUg dG8gcHJpbnRJbnRlcm5hbCgpIG1ldGhvZHMgb2YgUHJpbnRTdHJlYW0uaApkaWZmIC0tZ2l0IGEv U291cmNlL1dURi93dGYvQXRvbWljcy5oIGIvU291cmNlL1dURi93dGYvQXRvbWljcy5oCmluZGV4 IDc1MGUxMDkyYWQzN2E1N2UyNWUyZmRiNzI5MmZmYmQ1YjQzOWM3MDguLjg2ZWQ4MDVhMWUzYWRl NmY5NDJkMzZjNDc5MzZlZjdjMDc1NGE0NTQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvd3RmL0F0 b21pY3MuaAorKysgYi9Tb3VyY2UvV1RGL3d0Zi9BdG9taWNzLmgKQEAgLTExOCw4ICsxMTgsMTEg QEAgaW5saW5lIGludCBhdG9taWNEZWNyZW1lbnQoaW50IHZvbGF0aWxlKiBhZGRlbmQpIHsgcmV0 dXJuIF9fYXRvbWljX2RlYyhhZGRlbmQpIC0KICNlbGlmIENPTVBJTEVSKEdDQykgJiYgIUNQVShT UEFSQzY0KSAvLyBzaXplb2YoX0F0b21pY193b3JkKSAhPSBzaXplb2YoaW50KSBvbiBzcGFyYzY0 IGdjYwogI2RlZmluZSBXVEZfVVNFX0xPQ0tGUkVFX1RIUkVBRFNBRkVSRUZDT1VOVEVEIDEKIAot aW5saW5lIGludCBhdG9taWNJbmNyZW1lbnQoaW50IHZvbGF0aWxlKiBhZGRlbmQpIHsgcmV0dXJu IF9fZ251X2N4eDo6X19leGNoYW5nZV9hbmRfYWRkKGFkZGVuZCwgMSkgKyAxOyB9Ci1pbmxpbmUg aW50IGF0b21pY0RlY3JlbWVudChpbnQgdm9sYXRpbGUqIGFkZGVuZCkgeyByZXR1cm4gX19nbnVf Y3h4OjpfX2V4Y2hhbmdlX2FuZF9hZGQoYWRkZW5kLCAtMSkgLSAxOyB9CitpbmxpbmUgaW50IGF0 b21pY0luY3JlbWVudChpbnQgdm9sYXRpbGUqIGFkZGVuZCkgeyByZXR1cm4gX19zeW5jX2FkZF9h bmRfZmV0Y2goYWRkZW5kLCAxKTsgfQoraW5saW5lIGludCBhdG9taWNEZWNyZW1lbnQoaW50IHZv bGF0aWxlKiBhZGRlbmQpIHsgcmV0dXJuIF9fc3luY19zdWJfYW5kX2ZldGNoKGFkZGVuZCwgMSk7 IH0KKworaW5saW5lIGludDY0X3QgYXRvbWljSW5jcmVtZW50KGludDY0X3Qgdm9sYXRpbGUqIGFk ZGVuZCkgeyByZXR1cm4gX19zeW5jX2FkZF9hbmRfZmV0Y2goYWRkZW5kLCAxKTsgfQoraW5saW5l IGludDY0X3QgYXRvbWljRGVjcmVtZW50KGludDY0X3Qgdm9sYXRpbGUqIGFkZGVuZCkgeyByZXR1 cm4gX19zeW5jX3N1Yl9hbmRfZmV0Y2goYWRkZW5kLCAxKTsgfQogCiAjZW5kaWYKIAo=