Make LoadIndexedInstr for Uint32List use kUnboxedUint32 representation

Make LoadIndexedInstr for Uint32List use kUnboxedUint32 representation

BUG=
R=vegorov@google.com

Committed: https://code.google.com/p/dart/source/detail?r=40848

[Cutch, 2014-09-26 21:50:32]

johnmccutchan@google.com changed reviewers:
+ vegorov@google.com

[Vyacheslav Egorov (Google), 2014-09-30 15:06:02]

I suggest you do the same change for Int32 array and on all platforms. 

I think I implemented boxing/unboxing for i32/u32 on all of them.

https://codereview.chromium.org/612553002/diff/60001/runtime/vm/intermediate_...
File runtime/vm/intermediate_language_arm.cc (right):

https://codereview.chromium.org/612553002/diff/60001/runtime/vm/intermediate_...
runtime/vm/intermediate_language_arm.cc:1119: return Typed32BitIsSmi() ?
CompileType::FromCid(kSmiCid)
Any reason you don't want to support Int32List too?

https://codereview.chromium.org/612553002/diff/60001/runtime/vm/intermediate_...
runtime/vm/intermediate_language_arm.cc:1123: return
CompileType::FromCid(kMintCid);
This is not correct actually: boxing can result in both kSmiCid and kMintCid
which means this should be CompileType::Int() and ditto above for kMintCid case.

I wonder if we can create a regression test for this. Might be a bit hard
though.

[Cutch, 2014-09-30 15:39:12]

By all architectures do you mean 64-bit ones as well?

https://codereview.chromium.org/612553002/diff/60001/runtime/vm/intermediate_...
File runtime/vm/intermediate_language_arm.cc (right):

https://codereview.chromium.org/612553002/diff/60001/runtime/vm/intermediate_...
runtime/vm/intermediate_language_arm.cc:1119: return Typed32BitIsSmi() ?
CompileType::FromCid(kSmiCid)
On 2014/09/30 15:06:01, Vyacheslav Egorov (Google) wrote:
> Any reason you don't want to support Int32List too?

Done.

https://codereview.chromium.org/612553002/diff/60001/runtime/vm/intermediate_...
runtime/vm/intermediate_language_arm.cc:1123: return
CompileType::FromCid(kMintCid);
On 2014/09/30 15:06:02, Vyacheslav Egorov (Google) wrote:
> This is not correct actually: boxing can result in both kSmiCid and kMintCid
> which means this should be CompileType::Int() and ditto above for kMintCid
case.
> 
> I wonder if we can create a regression test for this. Might be a bit hard
> though.

Done. Here and above.

[Vyacheslav Egorov (Google), 2014-09-30 15:45:34]

Yes, I think for uniformity 64-bit architectures should be supported as well.
The code would not change afaik. As BoxInt32/BoxUint32 produces Smi there
already.

[Cutch, 2014-09-30 16:42:35]

PTAL

[Vyacheslav Egorov (Google), 2014-09-30 16:52:52]

lgtm

https://codereview.chromium.org/612553002/diff/100001/runtime/vm/intermediate...
File runtime/vm/intermediate_language_arm.cc (right):

https://codereview.chromium.org/612553002/diff/100001/runtime/vm/intermediate...
runtime/vm/intermediate_language_arm.cc:1300: 
Maybe make code look more like on ARM64?

https://codereview.chromium.org/612553002/diff/100001/runtime/vm/intermediate...
File runtime/vm/intermediate_language_ia32.cc (right):

https://codereview.chromium.org/612553002/diff/100001/runtime/vm/intermediate...
runtime/vm/intermediate_language_ia32.cc:1152: 
I suggest to make it look more like on ARM64 to reduce duplication.

if (representation() == kUnboxedInt32 ||
    representation() == kUnboxedUint32) {
  // Add assertion here in form of the switch if you feel it's necessary.
  Register result = locs()->out(0).reg();
  if ((index_scale() == 1) && index.IsRegister()) {
    __ SmiUntag(index.reg());
  }
  __ movl(result, element_address);
}

[Cutch, 2014-09-30 17:16:21]

https://codereview.chromium.org/612553002/diff/100001/runtime/vm/intermediate...
File runtime/vm/intermediate_language_arm.cc (right):

https://codereview.chromium.org/612553002/diff/100001/runtime/vm/intermediate...
runtime/vm/intermediate_language_arm.cc:1300: 
On 2014/09/30 16:52:51, Vyacheslav Egorov (Google) wrote:
> Maybe make code look more like on ARM64?

Done.

https://codereview.chromium.org/612553002/diff/100001/runtime/vm/intermediate...
File runtime/vm/intermediate_language_ia32.cc (right):

https://codereview.chromium.org/612553002/diff/100001/runtime/vm/intermediate...
runtime/vm/intermediate_language_ia32.cc:1152: 
On 2014/09/30 16:52:52, Vyacheslav Egorov (Google) wrote:
> I suggest to make it look more like on ARM64 to reduce duplication.
> 
> if (representation() == kUnboxedInt32 ||
>     representation() == kUnboxedUint32) {
>   // Add assertion here in form of the switch if you feel it's necessary.
>   Register result = locs()->out(0).reg();
>   if ((index_scale() == 1) && index.IsRegister()) {
>     __ SmiUntag(index.reg());
>   }
>   __ movl(result, element_address);
> }

Done.

[Cutch, 2014-10-01 17:46:55]

Committed patchset #8 (id:140001) manually as 40848 (presubmit successful).