Closed
Bug 681745
Opened 13 years ago
Closed 13 years ago
IonMonkey: Implement MTableSwitch(MIRType_Double)
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: dvander, Assigned: dvander)
References
Details
Attachments
(1 file)
24.37 KB,
patch
|
sstangl
:
review+
h4writer
:
review+
|
Details | Diff | Splinter Review |
This is needed for MTableSwitch.
Assignee | ||
Updated•13 years ago
|
Summary: IonMonkey: Implement MToInt32(MIRType_Double) → IonMonkey: Implement MTableSwitch(MIRType_Double)
Assignee | ||
Comment 1•13 years ago
|
||
Most of this patch is refactoring of bailoutIf, so emitDoubleToInt32 can be used for both deopts and safe failures (like for table switch). I could go back to the LDoubleToInt32 solution, which would (unnecessarily) deoptimize if switch encountered a double, if that seemed simpler, but I can't yet see if it would have any other consumers.
Attachment #555575 -
Flags: review?(sstangl)
Assignee | ||
Comment 2•13 years ago
|
||
Comment on attachment 555575 [details] [diff] [review] fix Hannes, could you look at the table switch changes? * It now uses the COPY policy when it can. * Took out the LIR-level constant folding - mostly because it was about to get more complicated, so maybe if it ends up being important we should look at ways to fold constant branches in MIR. But I can add it back if you want.
Attachment #555575 -
Flags: review?(hv1989)
Comment 3•13 years ago
|
||
Comment on attachment 555575 [details] [diff] [review] fix Review of attachment 555575 [details] [diff] [review]: ----------------------------------------------------------------- Seems all right. About the constant folding: - I think constant in a switch won't occur that much. I only see people that are testing doing that. So it's not that big of a deal. - From what I see the code wouldn't get that much more complicated. (Just a double(int(index)) == index_d check if type is double) - Adding it to MIR would work but would be very very hacky!
Attachment #555575 -
Flags: review?(hv1989) → review+
Comment 4•13 years ago
|
||
Comment on attachment 555575 [details] [diff] [review] fix Review of attachment 555575 [details] [diff] [review]: ----------------------------------------------------------------- This seems fine. I am unclear on how this logic would prettily transfer to permit handling switches with double-valued cases, but I suspect it is not a meaningful issue. ::: js/src/ion/TypePolicy.cpp @@ +270,5 @@ > MDefinition *in = ins->getOperand(0); > MInstruction *replace; > > // Tableswitch can consume all types, except: > // - Double: try to convert to int32 Can remove this comment now. ::: js/src/ion/shared/CodeGenerator-x86-shared.cpp @@ +463,5 @@ > > + if (ins->index()->isDouble()) { > + temp = ins->tempInt(); > + > + // The input is a double, so try and convert it to an integer. If it "If it" to line below. @@ +464,5 @@ > + if (ins->index()->isDouble()) { > + temp = ins->tempInt(); > + > + // The input is a double, so try and convert it to an integer. If it > + // does not fit in an integer, take the default case. Note that this particular implementation of TableSwitch is limited to only those switches with integer-valued cases.
Attachment #555575 -
Flags: review?(sstangl) → review+
Assignee | ||
Comment 5•13 years ago
|
||
http://hg.mozilla.org/projects/ionmonkey/rev/a4e9fd62263d and another checkin for follow-up nits
Assignee | ||
Updated•13 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•