Closed Bug 627299 Opened 10 years ago Closed 10 years ago

Assembler arguments not passed in CFLAGS

Categories

(Firefox Build System :: General, defect)

ARM
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX
mozilla2.0b11

People

(Reporter: glandium, Assigned: glandium)

Details

Attachments

(3 files, 1 obsolete file)

In configure.in, there are several constructs like this:
-Wa, -msomething
The space makes these constructs useless and don't let the options go to the assembler.
Attachment #505360 - Flags: review?(khuey)
Does js/src have the same problem?
(In reply to comment #1)
> Does js/src have the same problem?

As a matter of fact, it does, and so does nsprpub.
Consider this r=me to fix js/src.  wtc will need to review nsprpub changes though.
(In reply to comment #2)
> (In reply to comment #1)
> > Does js/src have the same problem?
> 
> As a matter of fact, it does, and so does nsprpub.

The same patch applies to js/src:
$ patch js/src/configure.in < .hg/patches/asm-args
patching file js/src/configure.in
Hunk #1 succeeded at 4675 (offset -2485 lines).

nsprpub has some differences in context.
Extended to include js/src. Carrying over r+ per comment 3
Assignee: nobody → mh+mozilla
Attachment #505360 - Attachment is obsolete: true
Attachment #505408 - Flags: review+
Comment on attachment 505409 [details] [diff] [review]
Fix assembler arguments passed in CFLAGS. nsprpub version (checked in)

Patch checked in on the NSPR trunk (NSPR 4.8.8).

Checking in configure;
/cvsroot/mozilla/nsprpub/configure,v  <--  configure
new revision: 1.292; previous revision: 1.291
done
Checking in configure.in;
/cvsroot/mozilla/nsprpub/configure.in,v  <--  configure.in
new revision: 1.295; previous revision: 1.294
done
Attachment #505409 - Attachment description: Fix assembler arguments passed in CFLAGS. nsprpub version → Fix assembler arguments passed in CFLAGS. nsprpub version (checked in)
Attachment #505409 - Flags: review?(wtc) → review+
Comment on attachment 505409 [details] [diff] [review]
Fix assembler arguments passed in CFLAGS. nsprpub version (checked in)

Note: mozilla/nsprpub/configure.in doesn't have the MOZ_ARM_ARCH
code in this patch, so I made the changes manually, by searching
for "-Wa, " and removing the spaces.
(In reply to comment #8)
> Comment on attachment 505409 [details] [diff] [review]
> Fix assembler arguments passed in CFLAGS. nsprpub version (checked in)
> 
> Note: mozilla/nsprpub/configure.in doesn't have the MOZ_ARM_ARCH
> code in this patch, so I made the changes manually, by searching
> for "-Wa, " and removing the spaces.

The version in m-c has, which is what I got the patch against. This was added in bug 618789.
Yeah, I backed that out because it landed without review from a nspr peer.
Comment on attachment 505408 [details] [diff] [review]
Fix assembler arguments passed in CFLAGS

This fixes correctness of flags passing for the assembler on arm, and might fix bug 616413.
Attachment #505408 - Flags: approval2.0?
Attachment #505408 - Flags: approval2.0? → approval2.0+
http://hg.mozilla.org/mozilla-central/rev/32542df68da3
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b11
Backed out because of maemo bustage.

http://hg.mozilla.org/mozilla-central/rev/94a51a3b64d4

In the end we might just remove the whole thing in bug 626035, so I'm WONTFIXing...
Resolution: FIXED → WONTFIX
This backout also reduced Ts by about 100ms
(In reply to comment #14)
> This backout also reduced Ts by about 100ms

... which doesn't make *any* sense.
The -Wa flags are unnecessary (see vlad's comment in
bug 632162 comment 1) and can be harmful (see
comment 13).  So they should be removed.

Patch checked in on the NSPR trunk (NSPR 4.8.8).

Checking in configure;
/cvsroot/mozilla/nsprpub/configure,v  <--  configure
new revision: 1.296; previous revision: 1.295
done
Checking in configure.in;
/cvsroot/mozilla/nsprpub/configure.in,v  <--  configure.in
new revision: 1.299; previous revision: 1.298
done
Attachment #518990 - Flags: review?(vladimir)
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.