Closed Bug 1045948 Opened 5 years ago Closed 5 years ago

IonMonkey: LAllocation and LDefinition changes

Categories

(Core :: JavaScript Engine: JIT, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: sunfish, Assigned: sunfish)

Details

Attachments

(4 files)

The following is a series of patches which simplify LAllocation and LDefinition that I made while working on register allocation code.

This first patch makes use of pointer alignment to store the LAllocation::Kind bits, rather than using a dedicated tag flag. This has the downside of limiting the Kind field to 3 bits, but it has the upside of freeing up a bit for other purposes, and it simplifies the kind() function.
Attachment #8464391 - Flags: review?(bhackett1024)
This patch eliminates LDefinition::PASSTHROUGH and replaces it with the pre-existing mechanism for bogus defs. A bogus def means that an instruction is not using one of its statically provisioned outputs. Unlike PASSTHROUGH, a bogus def doesn't need to hold on to a virtual register, which makes it a little easier to think about.
Attachment #8464402 - Flags: review?(bhackett1024)
This patch changes the representation of bogus LAllocations from being a special LConstantIndex (which represents bogus when used in a context which otherwise couldn't use an LConstantIndex) to being a default-constructed LAllocation, and adds an isBogus() predicate.

This also cleans up some cases where isUse() was used to test for a default-allocated LAllocation, as LAllocations now just default to a bogus state.
Attachment #8464403 - Flags: review?(bhackett1024)
This patch makes LAllocation::data() unsigned, since it's essentially just a container for bits.
Attachment #8464405 - Flags: review?(bhackett1024)
Attachment #8464391 - Flags: review?(bhackett1024) → review+
Comment on attachment 8464402 [details] [diff] [review]
eliminate-passthrough.patch

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

Nice, PASSTHROUGH never made sense to me.
Attachment #8464402 - Flags: review?(bhackett1024) → review+
Comment on attachment 8464403 [details] [diff] [review]
simplify-bogus.patch

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

::: js/src/jit/LIR.h
@@ +458,5 @@
>      }
>  
>      LDefinition() : bits_(0)
> +    {
> +        JS_ASSERT(isBogusTemp());

Can you assert isBogus in the LAllocation default constructor too?
Attachment #8464403 - Flags: review?(bhackett1024) → review+
Attachment #8464405 - Flags: review?(bhackett1024) → review+
(In reply to Brian Hackett (:bhackett) from comment #5)
> Comment on attachment 8464403 [details] [diff] [review]
> simplify-bogus.patch
> 
> Review of attachment 8464403 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/jit/LIR.h
> @@ +458,5 @@
> >      }
> >  
> >      LDefinition() : bits_(0)
> > +    {
> > +        JS_ASSERT(isBogusTemp());
> 
> Can you assert isBogus in the LAllocation default constructor too?

Done.

https://hg.mozilla.org/integration/mozilla-inbound/rev/14246833d209
https://hg.mozilla.org/integration/mozilla-inbound/rev/59aa0319941b
https://hg.mozilla.org/integration/mozilla-inbound/rev/8e45a9d7abfb
https://hg.mozilla.org/integration/mozilla-inbound/rev/cafba2a1b359
You need to log in before you can comment on or make changes to this bug.