Improve polymorphic loads on single slots.

Improve polymorphic loads on single slots.

If all property lookups for a polymorphic load actually result in the
same field index under all maps, we can actually emit a monomorphic load
that is guarded by a map check that verifies that the actual map is in
the set of handled maps. This also allows GVN to get rid of redundant
such map checks.

R=danno@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=11132

[danno, 2012-03-09 11:52:57]

First round

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

http://codereview.chromium.org/9616014/diff/2001/src/hydrogen-instructions.h#...
src/hydrogen-instructions.h:2054: class HCheckMapSet: public
HTemplateInstruction<1> {
Please merge with HCheckMap as discussed

http://codereview.chromium.org/9616014/diff/2001/src/hydrogen-instructions.h#...
src/hydrogen-instructions.h:2076: virtual bool DataEquals(HValue* other) {
Fast case single maps by just comparing b->map_set() == map_set()

http://codereview.chromium.org/9616014/diff/2001/src/hydrogen.cc
File src/hydrogen.cc (right):

http://codereview.chromium.org/9616014/diff/2001/src/hydrogen.cc#newcode3875
src/hydrogen.cc:3875: static bool ComputeStoredField(Handle<Map> type,
Rename to something like ComputeLoadStoreField

http://codereview.chromium.org/9616014/diff/2001/src/hydrogen.cc#newcode3979
src/hydrogen.cc:3979: for (int i = 0; i < types->length() && count <
kMaxStorePolymorphism; ++i) {
kMaxStorePolymorphism seems to be inappropriately named

[Michael Starzinger, 2012-03-12 11:37:11]

Added new patch set. PTAL.

https://chromiumcodereview.appspot.com/9616014/diff/2001/src/hydrogen-instruc...
File src/hydrogen-instructions.h (right):

https://chromiumcodereview.appspot.com/9616014/diff/2001/src/hydrogen-instruc...
src/hydrogen-instructions.h:2054: class HCheckMapSet: public
HTemplateInstruction<1> {
On 2012/03/09 11:52:58, danno wrote:
> Please merge with HCheckMap as discussed

Done. HCheckMap now works with a SmallMapList backing structure and has two
constructors, one taking a single map and the other a set of maps. Also added a
static helper to construct HCheckMaps which allow transitions.

https://chromiumcodereview.appspot.com/9616014/diff/2001/src/hydrogen-instruc...
src/hydrogen-instructions.h:2076: virtual bool DataEquals(HValue* other) {
On 2012/03/09 11:52:58, danno wrote:
> Fast case single maps by just comparing b->map_set() == map_set()

The backing SmallMapList cannot be compared by reference. But it already has all
the necessary fast-case code for singletons implemented. So I think relying on
the below loop being executed just once should be efficient enough.

https://chromiumcodereview.appspot.com/9616014/diff/2001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://chromiumcodereview.appspot.com/9616014/diff/2001/src/hydrogen.cc#newc...
src/hydrogen.cc:3875: static bool ComputeStoredField(Handle<Map> type,
On 2012/03/09 11:52:58, danno wrote:
> Rename to something like ComputeLoadStoreField

Done.

https://chromiumcodereview.appspot.com/9616014/diff/2001/src/hydrogen.cc#newc...
src/hydrogen.cc:3979: for (int i = 0; i < types->length() && count <
kMaxStorePolymorphism; ++i) {
On 2012/03/09 11:52:58, danno wrote:
> kMaxStorePolymorphism seems to be inappropriately named

Done. That actually should have been kMaxLoadPolymorphism in the first place.

[danno, 2012-03-16 16:41:29]

What about the x64 and ARM implementation?

http://codereview.chromium.org/9616014/diff/9001/src/ia32/lithium-codegen-ia3...
File src/ia32/lithium-codegen-ia32.cc (right):

http://codereview.chromium.org/9616014/diff/9001/src/ia32/lithium-codegen-ia3...
src/ia32/lithium-codegen-ia32.cc:4122: void LCodeGen::DoCheckMap(LCheckMap*
instr) {
Should this maybe be called DoCheckMaps now?

[Michael Starzinger, 2012-03-19 17:27:15]

Added new patch set. PTAL.

http://codereview.chromium.org/9616014/diff/9001/src/ia32/lithium-codegen-ia3...
File src/ia32/lithium-codegen-ia32.cc (right):

http://codereview.chromium.org/9616014/diff/9001/src/ia32/lithium-codegen-ia3...
src/ia32/lithium-codegen-ia32.cc:4122: void LCodeGen::DoCheckMap(LCheckMap*
instr) {
On 2012/03/16 16:41:29, danno wrote:
> Should this maybe be called DoCheckMaps now?

Done.

[danno, 2012-03-21 08:34:36]

lgtm