Closed
Bug 857856
Opened 10 years ago
Closed 10 years ago
--enable-debug --disable-optimize --disable-ion --disable-methodjit build is broken
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla23
People
(Reporter: Waldo, Assigned: Waldo)
Details
Attachments
(1 file, 1 obsolete file)
2.08 KB,
patch
|
sstangl
:
review+
|
Details | Diff | Splinter Review |
Not that I particularly care, but I'm working on implementing a feature that will need JIT support, and I want to ensure it works correctly in non-JIT before touching the JIT portion at all.
Attachment #733118 -
Flags: review?(sstangl)
Comment 1•10 years ago
|
||
Comment on attachment 733118 [details] [diff] [review] Patch Review of attachment 733118 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/Makefile.in @@ -414,2 @@ > CPPSRCS += Logging.cpp > -endif Can be reverted if definitions are in header ::: js/src/methodjit/Logging.cpp @@ +141,5 @@ > + > +bool > +js::IsJaegerSpewChannelActive(JaegerSpewChannel channel) > +{ > +} Move these functions to header as "static inline bool", with "return false" in the non-ENABLE_METHODJIT_SPEW case
Attachment #733118 -
Flags: review?(sstangl)
Assignee | ||
Comment 2•10 years ago
|
||
So I think the real problem, at its most basic, was defining JS_METHODJIT_SPEW if DEBUG. Looking at configure.in, I think there's already logic to that effect anyway, so it seems pointless to have it. Removed that, left the don't-#ifdef-so-much changes in place, with static-inlines for the relevant methods in the non-spew case.
Attachment #733118 -
Attachment is obsolete: true
Attachment #733452 -
Flags: review?(sstangl)
Comment 3•10 years ago
|
||
Comment on attachment 733452 [details] [diff] [review] v2 Review of attachment 733452 [details] [diff] [review]: ----------------------------------------------------------------- This looks good. In order to preserve the existing logic that permits JS_METHODJIT_SPEW when !JS_METHODJIT but ENABLE_YARR_JIT, you'll also have to change configure.in:3270 to check $ENABLE_YARR_JIT in addition to $ENABLE_METHODJIT. I think that should be safe, but it wouldn't hurt to test various build configurations before landing. ::: js/src/methodjit/Logging.h @@ +47,5 @@ > > +#else > + > +static inline void JMCheckLogging() {} > +inline bool IsJaegerSpewChannelActive(JaegerSpewChannel channel) { return false; } static inline bool @@ +91,2 @@ > > +} // namespace js Thanks :)
Attachment #733452 -
Flags: review?(sstangl) → review+
Assignee | ||
Comment 4•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/04f2c0f0a220
Assignee: general → jwalden+bmo
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla23
Comment 5•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/04f2c0f0a220
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•