Fix GVN for polymorphic loads....

Fix GVN for polymorphic loads.

They must not be hoisted above the compare-map-and-branch.


Committed: http://code.google.com/p/v8/source/detail?r=6082

[Kevin Millikin (Chromium), 2010-12-20 12:34:34]

Two comments you should address, otherwise LGTM.

http://codereview.chromium.org/5971003/diff/1/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

http://codereview.chromium.org/5971003/diff/1/src/hydrogen-instructions.h#new...
src/hydrogen-instructions.h:2552: HLoadNamedField(HValue* object, bool
is_in_object, int offset)
I'd rather pass is_polymorphic to the constructor and get rid of
set_is_polymorphic completely.

The way it's written, it's effectively a default parameter value (slightly worse
because it's artificially mutable), and more verbose too.

http://codereview.chromium.org/5971003/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

http://codereview.chromium.org/5971003/diff/1/src/hydrogen.cc#newcode1460
src/hydrogen.cc:1460: if (instr->IsLoadNamedField() &&
You could accomplish essentially the same thing by not setting kUseGVN for
polymorphic named loads.  That seems nicer than a special case in the
implementation.

We would use the ability to share them via GVN, but I don't think we can do that
in practice anyway.

[fschneider, 2010-12-20 12:46:08]

I agree - there is currently no point in disabling only loop-invariant code
motion for these. We can just as well disable GVN for polymorpic loads.