Closed Bug 1195477 Opened 5 years ago Closed 5 years ago

pass -fno-integrated-as where appropriate for clang-on-ARM

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox50 fixed)

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: froydnj, Assigned: froydnj)

References

Details

Attachments

(2 files)

We have this great bit of code in configure.in:

http://mxr.mozilla.org/mozilla-central/source/configure.in#7924

but we don't seem to be using it anywhere.  It is needed, though, in a number of places (pixman, skia, ctypes, openmax, etc.), because a lot of asm takes advantage of GNU as features.  And clang's integrated assembler doesn't support those features.

So, the question is, should we:

1) find all the places that choke with the integrated assembler, and add -fno-integrated-as as appropriate; or
2) (the hammer) always use -fno-integrated-as when compiling with clang on ARM?

I like #1, personally, even if it winds up being more code, because we only slow down the places that need to be slowed down.  WDYT?
Flags: needinfo?(mh+mozilla)
I'd vote for #1.
Flags: needinfo?(mh+mozilla)
I assume this is why I had to hack around so much assembly for my iOS port. It'd be nice if we did the right thing somehow so we didn't have to have one-off workarounds.
A clang build doesn't compile everything yet, but I spot-checked things and I
believe these are the only places we need to care about.
Attachment #8759670 - Flags: review?(mh+mozilla)
The ARM assembly sources in libffi don't work well with clang's
integrated assembler, so disable the integrated assembler for libffi.
Attachment #8759671 - Flags: review?(mh+mozilla)
Assignee: nobody → nfroyd
Comment on attachment 8759670 [details] [diff] [review]
part 1 - use -fno-integrated-as for clang builds on ARM, moz.build changes

Review of attachment 8759670 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/cairo/libpixman/src/moz.build
@@ +25,5 @@
>              'pixman-arm-simd-asm.S',
>          ]
> +    if CONFIG['CLANG_CXX']:
> +        ASFLAGS += [
> +            '-fno-integrated-as',

I don't have a strong opinion here, but it looks like the documented flag is -no-integrated-as (without a f), although both work. FWIW, we're already using -no-integrated-as in security/nss/lib/freebl/Makefile (so, it's actually third-party-ish)
Attachment #8759670 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8759671 [details] [diff] [review]
part 2 - pass -fno-integrated-as to libffi when building with clang

Review of attachment 8759671 [details] [diff] [review]:
-----------------------------------------------------------------

Same remark as the other patch.
Attachment #8759671 - Flags: review?(mh+mozilla) → review+
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4f607b7635fd
part 1 - use -no-integrated-as for clang builds on ARM, moz.build changes; r=glandium
https://hg.mozilla.org/integration/mozilla-inbound/rev/45a6a74c46ea
part 2 - pass -no-integrated-as to libffi when building with clang; r=glandium
We intermittently hit a couple of failures, https://treeherder.mozilla.org/logviewer.html#?job_id=30551526&repo=mozilla-inbound and https://treeherder.mozilla.org/logviewer.html#?job_id=30552748&repo=mozilla-inbound and then the failure rate started to steeply climb, so I thought I'd see if that meant you needed a clobber. Clobbered and retriggered on your push, and got https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=45a6a74c46ea709d4e6af586e9227f8b626bf595&selectedJob=30544324&filter-searchStr=e285b452a595156411478865aa36b33b78dd6a4c so I figured that meant that you only succeeded without a clobber, and failed entirely with. Clobbered again, and backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/9c245b0c0aaf91fda68d421ed7c74f4ea968f70a
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0f7edd43d27e
part 1 - use -no-integrated-as for clang builds on ARM, moz.build changes; r=glandium
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c3a439aee1c
part 2 - pass -no-integrated-as to libffi when building with clang; r=glandium
https://hg.mozilla.org/mozilla-central/rev/0f7edd43d27e
https://hg.mozilla.org/mozilla-central/rev/7c3a439aee1c
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Blocks: 1283678
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.