https://codereview.chromium.org/179773002/diff/30001/src/x64/lithium-x64.cc
File src/x64/lithium-x64.cc (right):
https://codereview.chromium.org/179773002/diff/30001/src/x64/lithium-x64.cc#newcode202
src/x64/lithium-x64.cc:202: if (array_operation->IsDehoisted()) {
This doesn't work completely, if you have phi nodes that have an input
that are phi nodes, and _those_ are used as keys in
KeyedLoads/KeyedStores, you won't sign extend them and neither will the
load/store itself, so you get the wrong result.
A correct approach would be to recursively visit uses through phis until
all non-phi usages have been examined, but that could potentially
revisit phis and all their uses many times, once for each unique
definition that flows into a phi.
Better would be to allocate a bit vector of the size of the maximum
HValue id, and during Hydrogen->Lithium translation (i.e.. in
DoLoadKeyed/DoStoreKeyed in lithium-x64.cc) set the bits for each
store/load key HValue recursively through phis, setting a bit in the bit
vector as you go, but stopping if a bit is already set for the HValue.
Then MustSignExtendResult would simply return whether the bit is set for
this->hydrogen_value()->id(). This approach ensures linear complexity
and make sure that all generated values that are used in any path are
properly sign extended.
https://codereview.chromium.org/179773002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
