Created attachment 46239 [details] Use portable version of TCMalloc_SpinLock with g++ on ppc This might be a regression/problem in g++, but with version 4.2.4 webkit-gtk 1.1.18 fails to build on OpenBSD/macppc with the following error : "JavaScriptCore/wtf/TCSpinLock.h: In function 'void* TCMalloc_SystemAlloc(size_t, size_t*, size_t)': JavaScriptCore/wtf/TCSpinLock.h:112: error: 'asm' operand requires impossible reload" It chokes on the compilation of JavaScriptCore/wtf/TCSystemAlloc.cpp which includes this header. It is not OpenBSD-only, as it's been reported against g++ in Debian here : http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=438415 G++ is : $/usr/local/bin/eg++ -v Using built-in specs. Target: powerpc-unknown-openbsd4.6 Configured with: /usr/obj/ports/gcc-4.2.4/gcc-4.2.4/configure --with-gmp=/usr/local --verbose --program-transform-name=s,^,e, --disable-nls --disable-checking --with-system-zlib --disable-libmudflap --disable-libgomp --disable-tls --with-as=/usr/bin/as --with-ld=/usr/bin/ld --with-gnu-ld --with-gnu-as --enable-threads=posix --enable-wchar_t --enable-languages=c,c++,fortran,objc --enable-cpp --with-gnu-as --with-gnu-ld --enable-shared --prefix=/usr/local --sysconfdir=/etc --mandir=/usr/local/man --infodir=/usr/local/info Thread model: posix gcc version 4.2.4 Note that webkit-gtk 1.1.15.4 and previous versions built fine on OpenBSD/macppc. So far, two options seems to fix this : either --enable-optimizations=no globally on ppc, which turns on -O0, which makes this file build fine, or use the attached patch (used in OpenBSD's ports tree) which makes it use the portable version of struct TCMalloc_SpinLock.
Would you be willing to submit a patch for review, as described in <http://webkit.org/coding/contributing.html>?
Well that patch is correct, no ? set review flag to '?'.
Comment on attachment 46239 [details] Use portable version of TCMalloc_SpinLock with g++ on ppc Thanks for contributing a patch! Three problems: 1) Needs a ChangeLog. 2) I believe the code in question works find on Mac OS X on PowerPC, and this patch changes things so it won't use it any more on that platform. 3) Not clear this is the right solution to the problem. Is this a workaround for a compiler bug or a real issue. review- because of problems (1) and (2) at least.
Created attachment 46387 [details] use portable version of TCMalloc_SpinLock on OpenBSD/macppc as it triggers g++ bugs. Changelog handwritten, as i don't have prepare-changelog. <snip> Bug 33451 - error: 'asm' operand requires impossible reload" on OpenBSD/macppc/g++ 4.2.4 https://bugs.webkit.org/show_bug.cgi?id=33451 * wtf/TCSPinLock.h: use portable version of TCMalloc_SpinLock on OpenBSD/macppc as it triggers g++ bugs. <snip> Adding a new patch that only disables the asm version on OpenBSD. i don't know how it works on other oses, nor what version of bloody g++ they use. Regarding 3), yeah i know it probably workarounds a compiler bug, but given the mess g++ is i wont spend time on it. I applied the patch for OpenBSD, and it works for me.
Comment on attachment 46387 [details] use portable version of TCMalloc_SpinLock on OpenBSD/macppc as it triggers g++ bugs. You can hand write change log, but it has to be added to the file ChangeLog in the right format. You can just copy an existing change log entry. I think making this specific to OPENBSD is bizarre. It should be specific to the versions of GCC with the bug. But if you don’t have time to do it right, it might be OK to land it just like this. But we do need someone to write the change log entry as for all other patches.
Created attachment 46389 [details] Changelog entry Phew... feels like bureaucracy entered free software. Isn't changelog file auto-generated these days ? I don't know if it's specific to this version of g++, as http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=438415 states that errors happens with various different versions. Webkit 1.1.15.4 didnt have that issue.
> Phew... feels like bureaucracy entered free software. We need a single patch that can be reviewed and automatically landed, please see <http://webkit.org/coding/contributing.html> for more information. If you are asking someone else to do the work for you, you probably need to explain why they would want to.
Fwiw, that patch is still needed with gcc 4.2.1 and webkit-gtk 1.6.3.
Created attachment 131905 [details] use portable version of TCMalloc_SpinLock on OpenBSD/macppc as it triggers g++ bugs. So, lets retry now that i finally got around getting an svn checkout of webkit & prepare-Changelog.... since that fix is still needed.
I've digged a bit more while updating to 1.8.l0, and that patch is still needed for OpenBSD/ppc. With pristine source (ie =o asm codepath) i get : Source/JavaScriptCore/wtf/TCSpinLock.h:113: error: 'asm' operand requires impossible reload Note that line 113 is on 'memory' line, not 'lockword'... I've searched history (esp https://bugs.webkit.org/show_bug.cgi?id=14521 and https://bugs.webkit.org/show_bug.cgi?id=17449) and i've tried making it take the other codepath (ie : "=m" (lockword_) ) and it failed with more vomit: c++ -DHAVE_CONFIG_H -I. -Wall -W -Wcast-align -Wchar-subscripts -Wreturn-type -Wformat -Wformat-security -Wno-format-y2k -Wundef -Wmissing-format-attribute -Wpointer-arith -Wwrite-strings -Wno-unused-parameter -Wno-parentheses -fno-exceptions -DENABLE_GLIB_SUPPORT=1 -DBUILDING_CAIRO__=1 -DBUILDING_GTK__=1 -DWTF_CHANGES -DXP_UNIX -DWTF_USE_ICU_UNICODE=1 -DWTF_USE_GSTREAMER=1 -DGTK_API_VERSION_2=1 -DNDEBUG -I./Source -I./Source/JavaScriptCore -I./Source/JavaScriptCore/API -I./Source/JavaScriptCore/assembler -I./Source/JavaScriptCore/bytecode -I./Source/JavaScriptCore/bytecompiler -I./Source/JavaScriptCore/dfg -I./Source/JavaScriptCore/heap -I./Source/JavaScriptCore/debugger -I./Source/JavaScriptCore/ForwardingHeaders -I./Source/JavaScriptCore/interpreter -I./Source/JavaScriptCore/jit -I./Source/JavaScriptCore/jit -I./Source/JavaScriptCore/parser -I./Source/JavaScriptCore/profiler -I./Source/JavaScriptCore/runtime -I./Source/JavaScriptCore/tools -I./Source/JavaScriptCore/wtf -I./Source/JavaScriptCore/wtf -I./Source/JavaScriptCore/wtf/gobject -I./Source/JavaScriptCore/wtf/gtk -I./Source/JavaScriptCore/wtf/text -I./Source/JavaScriptCore/wtf/unicode -I./Source/JavaScriptCore/yarr -I./DerivedSources/JavaScriptCore -I./Source/WTF -I/usr/local/include/libpng -I/usr/local/include -I/usr/X11R6/include -fno-rtti -fstrict-aliasing -O3 -pthread -I/usr/local/include/glib-2.0 -I/usr/local/lib/glib-2.0/include -I/usr/local/include -D_REENTRANT -I/usr/local/include -O2 -pipe -Wall -MT Source/JavaScriptCore/wtf/libjav ascriptcoregtk_1_0_la-FastMalloc.lo -MD -MP -MF Source/JavaScriptCore/wtf/.deps/libjavascriptcoregtk_1_0_la-FastMalloc.Tpo -c Source/JavaScriptCore/wtf/FastMalloc.cpp -fPIC -DPIC -o Source/JavaScriptCore/wtf/.libs/libjavascriptcoregtk_1_0_la-FastMalloc.o In file included from /usr/include/g++/iosfwd:45, from /usr/include/g++/bits/stl_algobase.h:70, from /usr/include/g++/algorithm:65, from Source/JavaScriptCore/wtf/FastMalloc.cpp:422: /usr/include/g++/powerpc-unknown-openbsd5.1/bits/c++locale.h: In function 'int std::__convert_from_v(int* const&, char*, int, c onst char*, ...)': /usr/include/g++/powerpc-unknown-openbsd5.1/bits/c++locale.h:81: warning: function might be possible candidate for 'printf' for mat attribute {standard input}: Assembler messages: {standard input}:1561: Error: syntax error; found `,' but expected `(' {standard input}:1561: Error: junk at end of line: `,24' {standard input}:1578: Error: syntax error; found `,' but expected `(' {standard input}:1578: Error: junk at end of line: `,22' {standard input}:1607: Error: syntax error; found `,' but expected `(' {standard input}:1607: Error: junk at end of line: `,24' {standard input}:5280: Error: syntax error; found `,' but expected `(' {standard input}:5280: Error: junk at end of line: `,11' So it seems either way i still need to use the portable version..
Created attachment 134871 [details] Assembly output from FastMalloc.cpp, when using : "=m" (lockword_) Looking at the asm the 4 offending instructions are stw with 3 numeric args(stw 27, 28,24), while others have two args (f .ex stw 0,-360(29))
Comment on attachment 131905 [details] use portable version of TCMalloc_SpinLock on OpenBSD/macppc as it triggers g++ bugs. I would cq+ this, but it appears the patch no longer applies.
It seems the offending asm code has disappeared in r120356 / bug 88957, and TCSpinLock.h is now using WTF::memoryBarrierAfterLock & WTF::memoryBarrierBeforeUnlock from Atomics.h. I dont see specific ppc code there anymore. I know the patch was needed for webkitgtk 1.8.x, but it's not needed anymore for 1.10.x. So maybe disregard that issue.