Closed Bug 677380 Opened 13 years ago Closed 13 years ago

IonMonkey: Refactor Lowering

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dvander, Assigned: dvander)

References

Details

Attachments

(2 files)

The lowering and codegen phases are closely related, but the organization of the two is completely different. For Codegen, the model is:

  shared
    \
     ---- x86-shared
            \
             ---- x86
            \
             ---- x64
    \
     ---- arm

  (x86, x64, arm)
    \
     ---- CodeGenerator

This makes it really easy to choose what level some functionality should be shared at. For the lowering phase, though, we have:

  Lowering
    \
     ---- Lowering-x86
    \
     ---- Lowering-x64
    \
     ---- Lowering-arm

There's no way to share between x86 and x64, and most of the visitors end up in Lowering which then performs virtual calls into Lowering-*.

On the other hand, the scale at how differently two things lower on a platform is either major or miniscule. For example, LAddI on x86/x64 need to reuse their first input, but ARM does not. LTruncateDoubleToInt32 needs an extra temporary on ARM. On the other extreme, an UnboxDouble operation looks entirely different on every CPU.

I'm not sure what the best organization is here, but as a start I'm going to make the Lowering organization look more like CodeGenerator and MacroAssembler.
Get rid of Ion* prefix on these files, since nothing else in the tree shares the same name.
Attachment #551645 - Flags: review?(sstangl)
Split off classes so we now have:

 Lowering-shared: this contains the most basic essentials for lowering.
 Lowering-x86/x64: platform specific stuff, derives from shared.
 Lowering: derives from x86/x64, contains common stuff that may depend on
           generic details provided by the platform
Attachment #551646 - Flags: review?(sstangl)
Comment on attachment 551645 [details] [diff] [review]
part 1: rename IonLowering -> Lowering

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

We should do this for other files and classes too. The redundancy was bothering me.
Attachment #551645 - Flags: review?(sstangl) → review+
Comment on attachment 551646 [details] [diff] [review]
part 2: reorganize

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

::: js/src/ion/shared/Lowering-shared.h
@@ +141,5 @@
> +    bool visitConstant(MConstant *ins);
> +};
> +
> +} // namespace js
> +} // namespace ion

nit: these are backwards.
Attachment #551646 - Flags: review?(sstangl) → review+
You need to log in before you can comment on or make changes to this bug.