On 2012/04/13 12:49:33, Sven Panne wrote:
http://codereview.chromium.org/10032029/diff/4001/src/hydrogen.cc
File src/hydrogen.cc (right):
http://codereview.chromium.org/10032029/diff/4001/src/hydrogen.cc#newcode2671
src/hydrogen.cc:2671: added_index_->InsertBefore(added_simulate);
On 2012/04/13 12:38:00, fschneider wrote:
> I think this is a little problematic and can be simplified: You only
need an
> HSimulate if the HAdd has side-effects (i.e. if it is a tagged-add). In
this
> case it is not safe to move it around in any case, since it can have
side
> effects.)
>
> If you limit yourself to index expressions a[i+c] where the index i+c
is an
> integer add (hadd->representation()->IsInteger32()), then you don't
need to
> worry about the HSimulate at all and you can insert it anywhere without
creating
> a new HSimulate.
Good point, but I suggest using 'foo->HasObservableSideEffects()' instead
of
'foo->representation()->IsInteger32()'. The side effects are the real
reason
for
disallowing this, and it's more consistent with HInstruction::Verify().
Actually even simpler: I'd expect that you can ASSERT that, if the index
expression is an HAdd, that the HAdd has int32 representation since the fast
element load/store requires the index to be int32.
http://codereview.chromium.org/10032029/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
