Bug 71498 - JIT-specific code should be able to refer to register types even on JIT-disabled builds
Summary: JIT-specific code should be able to refer to register types even on JIT-disab...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-03 13:04 PDT by Filip Pizlo
Modified: 2011-11-03 13:23 PDT (History)
1 user (show)

See Also:


Attachments
the patch (1.55 KB, patch)
2011-11-03 13:08 PDT, Filip Pizlo
barraclough: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2011-11-03 13:04:53 PDT
Increasingly, code in CodeBlock and elsewhere outside of the JIT refers to JIT features.  This is part of the evolution of the system to rely more on high-performance JIT optimizations.  But we still want to support no-JIT builds.  Doing this often means that we must insert #if's in a bunch of places if no JIT support is present.  A particularly egregious case is the current build bug introduced by http://trac.webkit.org/changeset/99148.  That changeset makes use of MacroAssembler::RegisterID in structs and classes that are placed in CodeBlock.  In the case that the JIT is turned off, no instances of those classes are ever instantiated.  But we've taken the approach of letting them be declared without #if guards to minimize noise.

But this breaks because the latest addition to these classes is ValueRecovery (referenced from InlineCallFrame, which is referenced from CodeBlock's RareData), which uses MacroAssembler::RegisterID.  We can either fix this by adding #if's around all references from CodeBlock to ValueRecovery (transitively, of course), or we can localize the noise by making MacroAssembler.h define a dummy MacroAssembler, with a dummy RegisterID, even if the assembler (and the JIT) is disabled.  This will have no run-time overhead, since this would just be an empty class with some typedef's, and ValueRecovery is never instantiated in non-JIT builds anyway.
Comment 1 Filip Pizlo 2011-11-03 13:08:25 PDT
Created attachment 113545 [details]
the patch
Comment 2 Gavin Barraclough 2011-11-03 13:17:37 PDT
Comment on attachment 113545 [details]
the patch

Ick, a bit ugly, but I guess the alternative involves a bunch of ifdefing code / data structures holding these as fields?  Probably for the best.
Comment 3 Filip Pizlo 2011-11-03 13:23:45 PDT
Landed in http://trac.webkit.org/changeset/99232