IM: LICM - hoist loop invariant instructions

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Andrew Scheff, Unassigned)

Tracking

(Depends on: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
Created attachment 543284 [details] [diff] [review]
This patch adds code hoisting functionality
Attachment #543284 - Flags: review?(dvander)
(Reporter)

Updated

6 years ago
No longer blocks: 664547
(Reporter)

Updated

6 years ago
Blocks: 664547
Comment on attachment 543284 [details] [diff] [review]
This patch adds code hoisting functionality

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

Looks good, r=me with changes

::: js/src/ion/LICM.h
@@ +102,5 @@
> +    // only if they allow another instruction that is a BIG_WIN to be hoisted as well
> +    static const int NO_WIN = 0;
> +    static const int POTENTIAL_WIN = 1;
> +    static const int BIG_WIN = 2;
> +    int getHoistWin(MInstruction *ins);

Make this an enum instead. Function name is a little confusing - how about "estimateHoistingWin"?

@@ +103,5 @@
> +    static const int NO_WIN = 0;
> +    static const int POTENTIAL_WIN = 1;
> +    static const int BIG_WIN = 2;
> +    int getHoistWin(MInstruction *ins);
> +    bool checkPotentialWin(MInstruction *ins);

How about "shouldHoist"?
Attachment #543284 - Flags: review?(dvander) → review+
(Reporter)

Comment 2

6 years ago
Created attachment 544090 [details] [diff] [review]
Improved code hoisting

Addresses comments, as well as:

Added enum to MInstruction, instructions are now classified by how they should be considered for hoisting

Added method that checks for block hotness, with the eventual goal of not hoisting instructions from cold blocks inside of loops.  How this will be done is TBD.
Attachment #543284 - Attachment is obsolete: true
Attachment #544090 - Flags: review?(dvander)
Comment on attachment 544090 [details] [diff] [review]
Improved code hoisting

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

::: js/src/ion/LICM.cpp
@@ +248,5 @@
> +
> +bool
> +Loop::checkHotness(MBasicBlock *block)
> +{
> +    // TODO

TODO should be a filed a bug w/ explanation, otherwise it will get forgotten :)
Attachment #544090 - Flags: review?(dvander) → review+
(Reporter)

Updated

6 years ago
Depends on: 669517
(Reporter)

Comment 4

6 years ago
Done, pushed.

http://hg.mozilla.org/users/danderson_mozilla.com/ionmonkey/rev/85d6d28605e5
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.