Minimize the #includes in js/src/jit

RESOLVED FIXED in mozilla27

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Ehsan, Assigned: Ehsan)

Tracking

(Blocks: 1 bug)

Trunk
mozilla27
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Comment 2

5 years ago
Comment on attachment 810705 [details] [diff] [review]
Patch (v1)

Thanks!  One nit throughout the patch: can you indent everywhere you put #idefs like so:

#ifdef MOZ_INSTRUMENTS
# include "devtools/Instruments.h"
#endif
Attachment #810705 - Flags: review?(luke) → review+
(Assignee)

Comment 3

5 years ago
(In reply to comment #2)
> Thanks!  One nit throughout the patch: can you indent everywhere you put #idefs
> like so:
> 
> #ifdef MOZ_INSTRUMENTS
> # include "devtools/Instruments.h"
> #endif

Sure, will do before landing.
(Assignee)

Comment 5

5 years ago
This broke no-ion builds: <https://tbpl.mozilla.org/php/getParsedLog.php?id=28432259&tree=Mozilla-Inbound>

I tried to fix this locally, but I don't think I know enough about what should and should not be in no-ion builds.  Luke, any idea who can fix those builds?
Flags: needinfo?(luke)

Comment 6

5 years ago
It's weird that this didn't show up in the try push.  You should be able to repro the compile errors locally by just configuring and building a js shell with --disable-ion.
Flags: needinfo?(luke)
https://hg.mozilla.org/mozilla-central/rev/5b35eb07b456
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Depends on: 921437

Comment 8

5 years ago
Note: I am seeing a lot of build errors with --disable-ion.
(Assignee)

Comment 9

5 years ago
(In reply to Luke Wagner [:luke] from comment #6)
> It's weird that this didn't show up in the try push.  You should be able to
> repro the compile errors locally by just configuring and building a js shell
> with --disable-ion.

Yeah I can *reproduce* the errors just fine.  I just don't know how to fix them, because it actually requires one to know what code should be compiled in no-ion mode and which code should not... :/
Flags: needinfo?(luke)
Created attachment 811901 [details] [diff] [review]
Fix --disable-ion build

I don't know why ForkJoin.h needs MIR.h and Clang doesn't know either (tried both debug/opt builds).
Attachment #811901 - Flags: review?(luke)

Updated

5 years ago
Attachment #811901 - Flags: review?(luke) → review+
Sorry for that. We have to forward-declare MDefinition, don't know how I missed that and why it only breaks Android.

I sent an updated patch to Try. On the bright side, it did fix the No-Ion build...
Let's try this again with a forward declaration of MDefinition:

https://hg.mozilla.org/integration/mozilla-inbound/rev/e43f1bac03c1

Linux/Windows/Android/Android NoIon are all green on Try:

https://tbpl.mozilla.org/?tree=Try&showall=1&rev=1a0bf267064e
Depends on: 923354
You need to log in before you can comment on or make changes to this bug.