Closed Bug 996422 Opened 10 years ago Closed 10 years ago

Split MIRType_Magic into one distinct type per magic constant

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: shu, Assigned: shu)

References

Details

Attachments

(2 files)

While discussing some bugs that part 1 of bug 716647, Jan suggested that the
MIRTypes for magic arguments vs the magic optimized out value have separate
types. This worked well, and got rid of a lot of nasty corner cases around
phi-merging magic constants. While rewriting that patch, I found it necessary
to refactor some TI and Ion stuff, so putting these refactor patches here.
Ion is the sole consumer of TI, so I see no reason to go from TI -> JSValueType
-> MIRType, especially since this means we can lose precision about the magic
arguments type: if the JSVAL_TYPE_ARGUMENTS came from TI, then it must mean
MIRType_MagicOptimizedArguments, since TI doesn't observe other magic types in
general. But if Ion tries to get the MIRType of some arbitrary magic value, it
shouldn't be MIRType_MagicOptimizedArguments.

This patches changes TI's JIT-oriented methods to work with MIRTypes directly
instead of via JSValueType.
Attachment #8406620 - Flags: review?(bhackett1024)
Attachment #8406620 - Flags: feedback?(jdemooij)
Actually does the split inside Ion.
Attachment #8406621 - Flags: review?(jdemooij)
Blocks: 716647
Attachment #8406620 - Flags: review?(bhackett1024) → review+
Comment on attachment 8406620 [details] [diff] [review]
Make TypeSets work with MIRTypes directly

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

I wasn't sure about this at first, but after looking at the patch it's nice to get rid of these JSValueType <-> MIRType conversions everywhere.
Attachment #8406620 - Flags: feedback?(jdemooij) → feedback+
Comment on attachment 8406621 [details] [diff] [review]
Split MIRType_Magic into one type for each magic constant

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

Nice.
Attachment #8406621 - Flags: review?(jdemooij) → review+
Try: https://tbpl.mozilla.org/?tree=Try&rev=be33b96ad263 (bottom 2 commits)
https://hg.mozilla.org/mozilla-central/rev/7756408df0dd
https://hg.mozilla.org/mozilla-central/rev/ff7d23524ff6
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.