Closed
Bug 577531
Opened 14 years ago
Closed 14 years ago
Fix non-thumb2 builds on Android
Categories
(NSPR :: NSPR, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
4.8.7
People
(Reporter: mwu, Assigned: blassey)
References
Details
(Keywords: regression)
Attachments
(4 files)
1.07 KB,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
3.03 KB,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
10.67 KB,
patch
|
blassey
:
review+
|
Details | Diff | Splinter Review |
2.35 KB,
patch
|
blassey
:
review+
dougt
:
approval2.0+
|
Details | Diff | Splinter Review |
Bug 563751 regressed non-thumb2 builds and as a result, non-thumb2 builds don't work on the Android emulator. This patch restores the CFLAGS/CXXFLAGS that make it work.
Attachment #456474 -
Flags: review?(ted.mielczarek)
Assignee | ||
Updated•14 years ago
|
Attachment #456474 -
Attachment is patch: true
Attachment #456474 -
Attachment mime type: application/octet-stream → text/plain
Updated•14 years ago
|
Attachment #456474 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Comment 1•14 years ago
|
||
perhaps we should have these flags only for non-thumb builds
Reporter | ||
Comment 2•14 years ago
|
||
(In reply to comment #1) > perhaps we should have these flags only for non-thumb builds Maybe. -mthumb-interwork does increase code size, according to the gcc documentation. This patch just makes things consistent with the other configure.ins.
Keywords: checkin-needed
Assignee | ||
Comment 4•14 years ago
|
||
i'd prefer to fix it with something like this so we don't have -march=armv5te and -march=armv7 at the same time. Also -mthumb-interwork presumably produces larger, sightly slower code, so let's not use it if we don't have to.
Attachment #465298 -
Flags: review?(ted.mielczarek)
Reporter | ||
Comment 5•14 years ago
|
||
(In reply to comment #4) > Created attachment 465298 [details] [diff] [review] > patch > > i'd prefer to fix it with something like this so we don't have -march=armv5te > and -march=armv7 at the same time. That's not really a problem. Gcc takes the last one you specify, and we already do this everywhere else. > Also -mthumb-interwork presumably produces > larger, sightly slower code, so let's not use it if we don't have to. This is somewhat risky I think. We have thumb-interwork on everywhere else. Have you tested with thumb-interwork turned off elsewhere in the code? I would want to make sure we can do this safely because I think the removal of this flag is what actually causes failure to run on the emulator. Maybe the system libraries use thumb..
Reporter | ||
Comment 6•14 years ago
|
||
(In reply to comment #5) > > Also -mthumb-interwork presumably produces > > larger, sightly slower code, so let's not use it if we don't have to. > This is somewhat risky I think. We have thumb-interwork on everywhere else. > Have you tested with thumb-interwork turned off elsewhere in the code? I would > want to make sure we can do this safely because I think the removal of this > flag is what actually causes failure to run on the emulator. Maybe the system > libraries use thumb.. Hm, nevermind, I see that it's left on for armv5. If removing it elsewhere doesn't break things, lets do it.
Assignee | ||
Comment 7•14 years ago
|
||
With these patches I've built with thumb2 enabled and disabled and both run fine on the nexus one. The build with --disable-thumb2 runs normally on the emulator.
Comment 8•14 years ago
|
||
Comment on attachment 465298 [details] [diff] [review] patch You'll need to split out the NSPR patch if you want me to land that for you.
Attachment #465298 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Comment 9•14 years ago
|
||
Attachment #468624 -
Flags: review+
Assignee | ||
Comment 10•14 years ago
|
||
Attachment #468625 -
Flags: review+
Reporter | ||
Updated•14 years ago
|
Assignee: mwu → blassey.bugs
Comment 11•14 years ago
|
||
Landed the NSPR patch in NSPR CVS: Checking in configure; /cvsroot/mozilla/nsprpub/configure,v <-- configure new revision: 1.287; previous revision: 1.286 done Checking in configure.in; /cvsroot/mozilla/nsprpub/configure.in,v <-- configure.in new revision: 1.291; previous revision: 1.290 done
Assignee | ||
Updated•14 years ago
|
Attachment #468625 -
Flags: approval2.0?
Comment 12•14 years ago
|
||
Comment on attachment 468625 [details] [diff] [review] m-c patch make sure this passed try.
Attachment #468625 -
Flags: approval2.0? → approval2.0+
Assignee | ||
Comment 13•14 years ago
|
||
pushed http://hg.mozilla.org/mozilla-central/rev/a2e5d3cbf6cf
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•13 years ago
|
Keywords: checkin-needed
Updated•13 years ago
|
Target Milestone: --- → 4.8.7
You need to log in
before you can comment on or make changes to this bug.
Description
•