Bug 124370 - Fix failed to build for Linux/MIPS64EL
Summary: Fix failed to build for Linux/MIPS64EL
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-14 10:14 PST by YunQiang Su
Modified: 2015-01-20 11:12 PST (History)
5 users (show)

See Also:

Attachments
patch for build webkit on Linux/MIPS64EL (2.70 KB, patch)
2013-11-14 10:15 PST, YunQiang Su 		no flags Details | Formatted Diff | Diff
patch merged into qtwebkit. (2.20 KB, patch)
2014-07-10 03:04 PDT, YunQiang Su 		no flags Details | Formatted Diff | Diff
Patch (2.46 KB, patch)
2014-07-10 05:40 PDT, Alberto Garcia 		no flags Details | Formatted Diff | Diff
Patch (2.46 KB, patch)
2015-01-18 11:57 PST, Alberto Garcia 		no flags Details | Formatted Diff | Diff
don't use __mips64 for N64 determination. (2.50 KB, patch)
2015-01-19 01:15 PST, YunQiang Su 		darin: review+
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description YunQiang Su 2013-11-14 10:14:46 PST 
There is no process for 64bit mips processors,
so for now, it will use JSVAULE32_64 instead of JSCVALUE64

With the attached patch, I can build webkitgtk (2.2.0) and
qtwebkit 2.2.1 on Debian Sid.
Comment 1 YunQiang Su 2013-11-14 10:15:46 PST 
Created attachment 216957 [details]
patch for build webkit on Linux/MIPS64EL
Comment 2 YunQiang Su 2014-07-10 03:02:34 PDT 
This patch has been merged into qtwebkit, and test well.

I also test it for webkitgtk.

Please consider merge it.
Comment 3 YunQiang Su 2014-07-10 03:04:51 PDT 
Created attachment 234697 [details]
patch merged into qtwebkit.
Comment 4 Alberto Garcia 2014-07-10 05:40:25 PDT 
Created attachment 234705 [details]
Patch

The patch needs a ChangeLog entry, I just added one.

Anyway I think the addition of new architectures must be discussed first in webkit-dev, am I right?
Comment 5 Geoffrey Garen 2014-07-11 14:25:18 PDT 
Using JSValue32_64 on a 64bit architecture is pretty weak sauce. (And does it work at all, given that pointers are 64bit?)

Is there anyone with enough involvement in the project, who cares about this architecture, who can make it work with JSValue64?
Comment 6 YunQiang Su 2014-07-11 20:01:06 PDT 
In this patch, we use JSValue64 not JSValue32_64.
Comment 7 YunQiang Su 2014-10-29 23:46:47 PDT 
Any progress of this bug?
Comment 8 Alberto Garcia 2014-11-02 11:21:08 PST 
This has been working in Debian for a while now, can anyone review this?
Comment 9 Alberto Garcia 2015-01-18 11:57:20 PST 
Created attachment 244865 [details]
Patch

I've just rebased the patch. Can someone please review?
Comment 10 YunQiang Su 2015-01-19 01:12:15 PST 
Sorry a problem discovered

#if defined(__mips64)

should be

defined(_MIPS_SIM_ABI64) && (_MIPS_SIM == _MIPS_SIM_ABI64)

as, __mips64 is also defined for N32 abi.
Comment 11 YunQiang Su 2015-01-19 01:15:23 PST 
Created attachment 244881 [details]
don't use __mips64 for N64 determination.
Comment 12 Alberto Garcia 2015-01-19 01:19:27 PST 
Comment on attachment 244881 [details]
don't use __mips64 for N64 determination.

Ok, thanks
Comment 13 Darin Adler 2015-01-20 09:27:12 PST 
Comment on attachment 244881 [details]
don't use __mips64 for N64 determination.

View in context: https://bugs.webkit.org/attachment.cgi?id=244881&action=review

I’m not a platform expert, but this patch looks OK to me.

> Source/WTF/wtf/Platform.h:85
> +#if (defined(mips) || defined(__mips__) || defined(MIPS) || defined(_MIPS_) \
> +    || defined(__mips64))

This line break just makes things hard to read. We are not trying to fit in 80 columns. Please consider putting this all on one line.
Comment 14 Alberto Garcia 2015-01-20 11:10:58 PST 
(In reply to comment #13)
> > +#if (defined(mips) || defined(__mips__) || defined(MIPS) || defined(_MIPS_) \
> > +    || defined(__mips64))
>
> This line break just makes things hard to read. We are not trying to
> fit in 80 columns. Please consider putting this all on one line.

Sure, I'll do that.

Thanks!
Comment 15 Alberto Garcia 2015-01-20 11:12:29 PST 
Committed r178725: <http://trac.webkit.org/changeset/178725>