WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
14521
JavaScriptCore fails to build on Linux/PPC gcc 4.1.2
https://bugs.webkit.org/show_bug.cgi?id=14521
Summary
JavaScriptCore fails to build on Linux/PPC gcc 4.1.2
Xan Lopez
Reported
2007-07-04 09:28:21 PDT
Ubuntu Fesity on PowerPC, gcc 4.1.2, WebKit from svn trunk as of 04/07/2007 19:29 EEST :) Output:../../../../JavaScriptCore/wtf/TCSpinLock.h: In function ‘void* TCMalloc_SystemAlloc(size_t, size_t)’: ../../../../JavaScriptCore/wtf/TCSpinLock.h:98: error: ‘asm’ operand requires impossible reload make[1]: *** [TCSystemAlloc.o] Error 1 make[1]: Leaving directory `/home/xan/svn/WebKit/WebKitBuild/Release/JavaScriptCore/kjs'
Attachments
Possible patch
(411 bytes, patch)
2007-11-11 09:11 PST
,
Mike Hommey
no flags
Details
Formatted Diff
Diff
0001-Use-less-strict-memory-operand-constraint-on-inline.patch
(1.59 KB, patch)
2007-12-18 09:45 PST
,
Xan Lopez
ggaren
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Patryk Zawadzki
Comment 1
2007-10-23 07:53:49 PDT
Same here with gcc-4.2.2 ppc-pld-linux-g++ -c -pipe -D_REENTRANT -D_REENTRANT -I/usr/include -O2 -fno-strict-aliasing -fwrapv -fsigned-char -fno-strict-aliasing -gdwarf-2 -g2 -Wall -W -DBUILDING_GTK__ -I/usr/share/qt4/mkspecs/linux-g++ -I../../../../JavaScriptCore/kjs -I../../../../JavaScriptCore -I../../../../JavaScriptCore/kjs -I../../../../JavaScriptCore/bindings -I../../../../JavaScriptCore/bindings/c -I../../../../JavaScriptCore/wtf -Itmp -I../../../../JavaScriptCore -I../../../../JavaScriptCore/kjs -I../../../../JavaScriptCore/bindings -I../../../../JavaScriptCore/bindings/c -I../../../../JavaScriptCore/wtf -I../../../../JavaScriptCore/pcre -Itmp -I../../../../JavaScriptCore/kjs -I. -o TCSystemAlloc.o ../../../../JavaScriptCore/wtf/TCSystemAlloc.cpp ../../../../JavaScriptCore/wtf/TCSpinLock.h: In function 'void* TCMalloc_SystemAlloc(size_t, size_t)': ../../../../JavaScriptCore/wtf/TCSpinLock.h:98: error: 'asm' operand requires impossible reload make[1]: *** [TCSystemAlloc.o] Error 1 make[1]: Leaving directory `/home/users/builder/rpm/BUILD/WebKit-
r26865
/WebKitBuild/Release/JavaScriptCore/kjs' make: *** [sub-JavaScriptCore-kjs-testkjs-pro-make_default-ordered] Error 2
David Kilzer (:ddkilzer)
Comment 2
2007-10-23 08:36:30 PDT
***
Bug 14974
has been marked as a duplicate of this bug. ***
David Kilzer (:ddkilzer)
Comment 3
2007-10-23 08:38:15 PDT
Details about how to work around this issue (using -O0 to compile this one file or switching to generic pthreads) is detailed here:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=438415
Bug 14974 Comment #2
Mike Hommey
Comment 4
2007-11-11 09:11:10 PST
Created
attachment 17187
[details]
Possible patch This patch solves the build problem on a simplified testcase, and produces the same assembly output in -O0 and -Os. I haven't checked if it builds with the real code, and can't verify it doesn't break at runtime, so if someone could check that out...
David Kilzer (:ddkilzer)
Comment 5
2007-11-11 12:43:14 PST
(In reply to
comment #4
)
> This patch solves the build problem on a simplified testcase, and produces the > same assembly output in -O0 and -Os. I haven't checked if it builds with the > real code, and can't verify it doesn't break at runtime, so if someone could > check that out...
Applying this patch and recompiling a local debug build of
r27683
seems to work on my PowerBook G4. JavaScriptCore tests and layout tests all pass. I don't know enough PPC assembly to give this an r+, though.
Xan Lopez
Comment 6
2007-11-11 12:46:57 PST
Works ok in Linux/PPC too (but no clue about its correctness either).
David Kilzer (:ddkilzer)
Comment 7
2007-11-11 12:57:45 PST
(In reply to
comment #5
)
> Applying this patch and recompiling a local debug build of
r27683
seems to work > on my PowerBook G4. JavaScriptCore tests and layout tests all pass. I don't > know enough PPC assembly to give this an r+, though.
The gcc inline assembly constraints are documented here:
http://www.ibiblio.org/gferg/ldp/GCC-Inline-Assembly-HOWTO.html#ss6.1
Here is the difference between "=m" and "=o" (the "=" is a separate modifier): "m" : A memory operand is allowed, with any kind of address that the machine supports in general. "o" : A memory operand is allowed, but only if the address is offsettable. ie, adding a small offset to the address gives a valid address.
Mike Hommey
Comment 8
2007-11-11 13:32:32 PST
FWIW, the PPC inline code has been added, according to the changelog, by mjs in revision 10701.
David Kilzer (:ddkilzer)
Comment 9
2007-11-11 14:09:13 PST
(In reply to
comment #7
)
> Here is the difference between "=m" and "=o" (the "=" is a separate modifier): > > "m" : A memory operand is allowed, with any kind of address that the machine > supports in general. > > "o" : A memory operand is allowed, but only if the address is offsettable. ie, > adding a small offset to the address gives a valid address.
Maciej, would changing "=o" to "=m" cause a significant performance issue? I'm sure there's a locality-of-reference issue here (where "=o" should perform better because it's going to be "closer"), but I'm not sure how much the affect would be. Is this something SunSpider would pick up?
Maciej Stachowiak
Comment 10
2007-11-11 19:04:55 PST
Comment on
attachment 17187
[details]
Possible patch There's two possible kinds of problems that can be caused by getting the assembly operand constraints wrong: 1) Inferior codegen, due to overconstraining the operands, and forcing gcc to do needless moves. 2) Incorrect codegen, de to using an operand that is disallowed. I think #2 will make the assembly stage fail, so if PPC compiles with this patch it should be safe. For #1, we should be ok as well, because m is a looser constraint than o, so you'd never have a needless copy to satisfy m when o is satisfied. r=me
Mark Rowe (bdash)
Comment 11
2007-11-11 22:03:31 PST
Landed in
r27708
.
Mark Rowe (bdash)
Comment 12
2007-11-11 22:20:56 PST
Rolled out in
r27709
as it broke the Mac PowerPC build: {standard input}:47313:Parameter syntax error (parameter 2) {standard input}:54379:Parameter syntax error (parameter 2)
David Kilzer (:ddkilzer)
Comment 13
2007-11-11 22:27:42 PST
(In reply to
comment #12
)
> Rolled out in
r27709
as it broke the Mac PowerPC build: > {standard input}:47313:Parameter syntax error (parameter 2) > {standard input}:54379:Parameter syntax error (parameter 2)
I only compiled a debug build with this change. I presume it failed on a release build? Does the buildbot system have an up-to-date version of Xcode installed?
Alexey Proskuryakov
Comment 14
2007-11-11 22:35:46 PST
(In reply to
comment #7
)
>
http://www.ibiblio.org/gferg/ldp/GCC-Inline-Assembly-HOWTO.html#ss6.1
Apple documentation: <
http://developer.apple.com/documentation/DeveloperTools/gcc-4.0.1/gcc/Simple-Constraints.html#Simple-Constraints
>
David Kilzer (:ddkilzer)
Comment 15
2007-11-11 22:37:35 PST
(In reply to
comment #13
)
> (In reply to
comment #12
) > > Rolled out in
r27709
as it broke the Mac PowerPC build: > > {standard input}:47313:Parameter syntax error (parameter 2) > > {standard input}:54379:Parameter syntax error (parameter 2) > > I only compiled a debug build with this change. I presume it failed on a > release build? Does the buildbot system have an up-to-date version of Xcode > installed?
I also get an error building a release build with this change, while the debug build worked fine. I have Xcode 2.4.1 installed. Compiler bug? $ gcc --version powerpc-apple-darwin8-gcc-4.0.1 (GCC) 4.0.1 (Apple Computer, Inc. build 5367) Copyright (C) 2005 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Mark Rowe (bdash)
Comment 16
2007-11-12 02:31:10 PST
Dave, if you have a PowerPC machine it may be worth creating a reduced test case and filing a bug report against the Apple compiler (assuming it works correctly with the same optimisations on for the standard GCC compiler?). We may need to #ifdef this to work around the issues.
Mike Hommey
Comment 17
2007-11-12 02:44:11 PST
FWIW, I have a simplified testcase for the original code here:
http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;filename=TCSystemAlloc.cpp;att=1;bug=438415
So you should only need to replace =o with =m.
David Kilzer (:ddkilzer)
Comment 18
2007-11-12 05:55:07 PST
(In reply to
comment #16
)
> Dave, if you have a PowerPC machine it may be worth creating a reduced test > case and filing a bug report against the Apple compiler (assuming it works > correctly with the same optimisations on for the standard GCC compiler?). We > may need to #ifdef this to work around the issues.
Will do. (In reply to
comment #17
)
> FWIW, I have a simplified testcase for the original code here: >
http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;filename=TCSystemAlloc.cpp;att=1;bug=438415
> > So you should only need to replace =o with =m.
Thanks, Mike! The OS X gcc compiler fails to compile AllInOneFile.cpp for Release builds, so I may have to start from there for that reduction. It's troubling that =o is broken for non-Apple gcc while =m is broken for Apple's gcc.
Darin Adler
Comment 19
2007-11-12 12:10:11 PST
Comment on
attachment 17187
[details]
Possible patch Clearing review flag.
David Kilzer (:ddkilzer)
Comment 20
2007-11-12 17:50:50 PST
(In reply to
comment #18
)
> (In reply to
comment #16
) > > Dave, if you have a PowerPC machine it may be worth creating a reduced test > > case and filing a bug report against the Apple compiler (assuming it works > > correctly with the same optimisations on for the standard GCC compiler?). We > > may need to #ifdef this to work around the issues. > > Will do.
Filed <
rdar://problem/5596043
> for the gcc issue. This does NOT cover this WebKit bug, so please don't add the "InRadar" keyword for this radar.
Mark Rowe (bdash)
Comment 21
2007-12-04 11:33:37 PST
***
Bug 16293
has been marked as a duplicate of this bug. ***
David Kilzer (:ddkilzer)
Comment 22
2007-12-04 14:26:03 PST
(In reply to
comment #20
)
> Filed <
rdar://problem/5596043
> for the gcc issue. This does NOT cover this > WebKit bug, so please don't add the "InRadar" keyword for this radar.
I think we should use #ifdefs to use "=o" on OS-X-compiled WebKit and "=m" for Linux-compiled WebKit (both using gcc). Perhaps something like: #if PLATFORM(DARWIN) : "=o" (private_lockword_) #else : "=m" (private_lockword_) #endif
Xan Lopez
Comment 23
2007-12-18 09:24:49 PST
(In reply to
comment #22
)
> Perhaps something like: > > #if PLATFORM(DARWIN) > : "=o" (private_lockword_) > #else > : "=m" (private_lockword_) > #endif >
This seems to work fine and sounds like a sensible compromise until the bug in the Apple compiler is fixed. Should I attach a proper patch with it?
David Kilzer (:ddkilzer)
Comment 24
2007-12-18 09:28:30 PST
(In reply to
comment #23
)
> This seems to work fine and sounds like a sensible compromise until the bug in > the Apple compiler is fixed. Should I attach a proper patch with it?
Yes, please do. Thanks!
Xan Lopez
Comment 25
2007-12-18 09:45:31 PST
Created
attachment 17975
[details]
0001-Use-less-strict-memory-operand-constraint-on-inline.patch Subject: [PATCH] Use less strict memory operand constraint on inline ASM for TCSpinLock.h. --- JavaScriptCore/ChangeLog | 15 +++++++++++++++ JavaScriptCore/wtf/TCSpinLock.h | 6 +++++- 2 files changed, 20 insertions(+), 1 deletions(-)
Geoffrey Garen
Comment 26
2007-12-18 10:26:15 PST
Comment on
attachment 17975
[details]
0001-Use-less-strict-memory-operand-constraint-on-inline.patch r=me, based on above comments
David Kilzer (:ddkilzer)
Comment 27
2007-12-18 18:59:25 PST
$ svn commit JavaScriptCore Sending JavaScriptCore/ChangeLog Sending JavaScriptCore/wtf/TCSpinLock.h Transmitting file data .. Committed revision 28847.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug