Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
22859007:
Insert dummy conversion instruction for impossible conversion requests. (Closed)
|
Created:
7 years, 4 months ago by Cutch Modified:
7 years, 4 months ago CC:
reviews_dartlang.org, vm-dev_dartlang.org Visibility:
Public. |
DescriptionInsert dummy conversion instruction for impossible conversion requests.
BUG=12417
R=srdjan@google.com
Committed: https://code.google.com/p/dart/source/detail?r=26143
Patch Set 1 #
Total comments: 8
Messages
Total messages: 9 (0 generated)
lgtm
Message was sent while issue was closed.
Committed patchset #1 manually as r26143 (presubmit successful).
Message was sent while issue was closed.
dbc: https://codereview.chromium.org/22859007/diff/1/runtime/vm/flow_graph_optimiz... File runtime/vm/flow_graph_optimizer.cc (right): https://codereview.chromium.org/22859007/diff/1/runtime/vm/flow_graph_optimiz... runtime/vm/flow_graph_optimizer.cc:408: converted = new UnboxDoubleInstr(use->CopyWithType(), deopt_id); How about inserting a Box- and an Unbox-instruction to avoid deoptimization here and in the other cases?
Message was sent while issue was closed.
https://codereview.chromium.org/22859007/diff/1/runtime/vm/flow_graph_optimiz... File runtime/vm/flow_graph_optimizer.cc (right): https://codereview.chromium.org/22859007/diff/1/runtime/vm/flow_graph_optimiz... runtime/vm/flow_graph_optimizer.cc:408: converted = new UnboxDoubleInstr(use->CopyWithType(), deopt_id); On 2013/08/15 09:11:35, Florian Schneider wrote: > How about inserting a Box- and an Unbox-instruction to avoid deoptimization here > and in the other cases? What John has observed is that this code is actually never executed. It occurs when inlining a function. The argument types are known and match one control flow, but other (never possible) control flows are not eliminated and legal code must still be generated for them as well (control flows are formed by is-a test on arguments). John may provide more information.
Message was sent while issue was closed.
https://codereview.chromium.org/22859007/diff/1/runtime/vm/flow_graph_optimiz... File runtime/vm/flow_graph_optimizer.cc (right): https://codereview.chromium.org/22859007/diff/1/runtime/vm/flow_graph_optimiz... runtime/vm/flow_graph_optimizer.cc:408: converted = new UnboxDoubleInstr(use->CopyWithType(), deopt_id); On 2013/08/15 09:11:35, Florian Schneider wrote: > How about inserting a Box- and an Unbox-instruction to avoid deoptimization here > and in the other cases? We are being asked to convert an UnboxedFloat32x4 to an UnboxedDouble. Even if we inserted a BoxFloat32x4Instr followed by an UnboxDoubleInstr a deoptimization will be triggered.
Message was sent while issue was closed.
https://codereview.chromium.org/22859007/diff/1/runtime/vm/flow_graph_optimiz... File runtime/vm/flow_graph_optimizer.cc (right): https://codereview.chromium.org/22859007/diff/1/runtime/vm/flow_graph_optimiz... runtime/vm/flow_graph_optimizer.cc:408: converted = new UnboxDoubleInstr(use->CopyWithType(), deopt_id); On 2013/08/15 14:35:02, Cutch wrote: > On 2013/08/15 09:11:35, Florian Schneider wrote: > > How about inserting a Box- and an Unbox-instruction to avoid deoptimization > here > > and in the other cases? > > We are being asked to convert an UnboxedFloat32x4 to an UnboxedDouble. Even if > we inserted a BoxFloat32x4Instr followed by an UnboxDoubleInstr a deoptimization > will be triggered. Still, I don't understand how this should work: Even if never executed, the representations still should match: Unbox-instructions need a boxed input (tagged) which comes in a regular register. An unboxed float32x4 value comes in a XMM register. https://codereview.chromium.org/22859007/diff/1/runtime/vm/flow_graph_optimiz... runtime/vm/flow_graph_optimizer.cc:410: converted = new UnboxDoubleInstr(use->CopyWithType(), deopt_id); Ok, but shouldn't this be UnboxUint32x4Instr here? https://codereview.chromium.org/22859007/diff/1/runtime/vm/flow_graph_optimiz... runtime/vm/flow_graph_optimizer.cc:412: converted = new UnboxDoubleInstr(use->CopyWithType(), deopt_id); Likewise, UnboxFloat32x4Instr here?
Message was sent while issue was closed.
https://codereview.chromium.org/22859007/diff/1/runtime/vm/flow_graph_optimiz... File runtime/vm/flow_graph_optimizer.cc (right): https://codereview.chromium.org/22859007/diff/1/runtime/vm/flow_graph_optimiz... runtime/vm/flow_graph_optimizer.cc:408: converted = new UnboxDoubleInstr(use->CopyWithType(), deopt_id); On 2013/08/16 10:31:18, Florian Schneider wrote: > On 2013/08/15 14:35:02, Cutch wrote: > > On 2013/08/15 09:11:35, Florian Schneider wrote: > > > How about inserting a Box- and an Unbox-instruction to avoid deoptimization > > here > > > and in the other cases? > > > > We are being asked to convert an UnboxedFloat32x4 to an UnboxedDouble. Even if > > we inserted a BoxFloat32x4Instr followed by an UnboxDoubleInstr a > deoptimization > > will be triggered. > > Still, I don't understand how this should work: Even if never executed, the > representations still should match: Unbox-instructions need a boxed input > (tagged) which comes in a regular register. An unboxed float32x4 value comes in > a XMM register. So what ends up happening is that unboxed -> tagged conversion routine is inserted anyways. I'm sending you a CL which makes this explicit and fixes the issue you found below. https://codereview.chromium.org/22859007/diff/1/runtime/vm/flow_graph_optimiz... runtime/vm/flow_graph_optimizer.cc:410: converted = new UnboxDoubleInstr(use->CopyWithType(), deopt_id); On 2013/08/16 10:31:18, Florian Schneider wrote: > Ok, but shouldn't this be UnboxUint32x4Instr here? Yes. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
