857856 - --enable-debug --disable-optimize --disable-ion --disable-methodjit build is broken

Status: RESOLVED FIXED

Description

[Jeff Walden [:Waldo]]

Created attachment 733118 [details] [diff] [review]
Patch

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.

Comment 1

[Sean Stangl [:sstangl]]

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

Comment 2

[Jeff Walden [:Waldo]]

Created attachment 733452 [details] [diff] [review]
v2

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.

Comment 3

[Sean Stangl [:sstangl]]

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 :)

Comment 4

[Jeff Walden [:Waldo]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/04f2c0f0a220

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/04f2c0f0a220