Closed Bug 1214562 Opened 4 years ago Closed 4 years ago

Cleanup SetPropertyCache/SetElementCache regalloc

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: jandem, Assigned: jandem)

References

Details

Attachments

(2 files)

Unifying them is a bit complicated due to their current regalloc contraints. I'll post some patches to clean this up.
LSetElementCacheV uses useByteOpRegister becaues that was needed for the typed array stub.

You fixed the x86 assembler to handle this more transparently so we no longer need this.
Attachment #8673589 - Flags: review?(bhackett1024)
This patch:

* Uses useBoxOrTypedOrConstant, so we can use a single LIR instruction for SetPropertyCache.

* Gives SetPropertyCache always a temp register instead of using a tempCopy. This will make bug 1214126 a lot nicer and simplifies many stubs because we need fewer stack pushes/pops.
Attachment #8673598 - Flags: review?(bhackett1024)
Comment on attachment 8673589 [details] [diff] [review]
Part 1 - Don't use useByteOpRegister

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

::: js/src/jit/Lowering.cpp
@@ +3527,5 @@
>  
>      // Due to lack of registers on x86, we reuse the object register as a
>      // temporary. This register may be used in a 1-byte store, which on x86
>      // again has constraints; thus the use of |useByteOpRegister| over
>      // |useRegister| below.

This comment should be fixed.
Attachment #8673589 - Flags: review?(bhackett1024) → review+
Comment on attachment 8673598 [details] [diff] [review]
Part 2 - Refactor SetPropertyCache

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

Nice!

::: js/src/jit/IonCaches.cpp
@@ +2239,5 @@
>      lastJump_ = initialJump_;
>  }
>  
>  // Jump to failure if a value being written is not a property for obj/id.
>  // This might clobber |object|.

This comment should be fixed.
Attachment #8673598 - Flags: review?(bhackett1024) → review+
https://hg.mozilla.org/mozilla-central/rev/0ea76f0263ef
https://hg.mozilla.org/mozilla-central/rev/73e1760e3c4d
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Depends on: 1222917
You need to log in before you can comment on or make changes to this bug.