Use GPRs for mints

Use GPRs for mints.

Changes:

* Register allocator now allocates GPRs for kUnboxedMint.
* Register allocator supports for SameAsFirstInput for register pairs.
* Register allocator properly handles register pairs in environment uses and materialization uses.
* BoxInteger updated on IA32/ARM.
* UnboxInteger updated on IA32/ARM.
* BinaryMintOp updated on IA32/ARM.
* ShiftMintOp updated on IA32/ARM.
* UnaryMintOp updated on IA32/ARM.
* RelationalOp updated on IA32/ARM.
* EqualityCompare updated on IA32/ARM.
* LoadIndexed and StoreIndexed updated on IA32/ARM.
* New Deopt instructions added.
* Update live_registers when an instruction has a fixed register input and a call on the slow path.
* Improve printing of register pairs in flow graph.
* Do not assume live registers in slow paths contain tagged values.
* LiveRange pairs for kUnboxedMint definitions marked as kUntagged representation (reduces stack usage).
* Live register spilling on ARM uses same register order as stack map encoding.
* Spill slots containing tagged and untagged are segregated.
* Print stack maps when printing live ranges with safe points.
* Print allocated spill slot when printing live ranges.

Status:

* IA32 completed. All tests are passing.
* ARM completed. All tests passing.

R=fschneider@google.com, srdjan@google.com, zra@google.com

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

[Cutch, 2014-04-30 18:25:40]

Architecture independent and IA32 is ready for review.

[srdjan, 2014-04-30 18:46:01]

Please also add tests.

https://codereview.chromium.org/252333002/diff/220001/runtime/vm/deopt_instru...
File runtime/vm/deopt_instructions.cc (right):

https://codereview.chromium.org/252333002/diff/220001/runtime/vm/deopt_instru...
runtime/vm/deopt_instructions.cc:724: static int64_t CombineLoAndHi(int32_t* lo,
int32_t* hi) {
Use utils.h LowHighTo64Bits

https://codereview.chromium.org/252333002/diff/220001/runtime/vm/deopt_instru...
runtime/vm/deopt_instructions.cc:737:
hi_reg_(static_cast<Register>(hi_reg_as_int)) {}
Assert int values are valid registers

https://codereview.chromium.org/252333002/diff/220001/runtime/vm/deopt_instru...
runtime/vm/deopt_instructions.cc:859: flip_(flip) {
ASSERT reg_as_int is a valid value.

https://codereview.chromium.org/252333002/diff/220001/runtime/vm/deopt_instru...
runtime/vm/deopt_instructions.cc:1513: // There are four cases to consider here:
Say R: Register, S: Stack-slot

[Florian Schneider, 2014-04-30 18:55:45]

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

https://codereview.chromium.org/252333002/diff/220001/runtime/vm/intermediate...
runtime/vm/intermediate_language_ia32.cc:371: __ movl(tmp, result_hi);
I think tmp is not needed anymore here.

https://codereview.chromium.org/252333002/diff/220001/runtime/vm/intermediate...
runtime/vm/intermediate_language_ia32.cc:5542: summary->set_temp(0,
Location::RequiresRegister());
A temp may not be needed anymore with your new code.

https://codereview.chromium.org/252333002/diff/220001/runtime/vm/intermediate...
runtime/vm/intermediate_language_ia32.cc:5690: summary->set_temp(0,
Location::RequiresRegister());
temp not needed here anymore.

https://codereview.chromium.org/252333002/diff/220001/runtime/vm/locations.cc
File runtime/vm/locations.cc (right):

https://codereview.chromium.org/252333002/diff/220001/runtime/vm/locations.cc...
runtime/vm/locations.cc:247: return Location::Pair(pair->At(0), pair->At(1));
I think here you should do a deep copy of the parts as well. Otherwise there
will be unintended sharing of the location parts.

[Cutch, 2014-05-14 17:16:19]

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

https://codereview.chromium.org/252333002/diff/220001/runtime/vm/intermediate...
runtime/vm/intermediate_language_ia32.cc:371: __ movl(tmp, result_hi);
On 2014/04/30 18:55:46, Florian Schneider wrote:
> I think tmp is not needed anymore here.

Done.

https://codereview.chromium.org/252333002/diff/220001/runtime/vm/intermediate...
runtime/vm/intermediate_language_ia32.cc:5542: summary->set_temp(0,
Location::RequiresRegister());
On 2014/04/30 18:55:46, Florian Schneider wrote:
> A temp may not be needed anymore with your new code.

Done.

https://codereview.chromium.org/252333002/diff/220001/runtime/vm/intermediate...
runtime/vm/intermediate_language_ia32.cc:5690: summary->set_temp(0,
Location::RequiresRegister());
On 2014/04/30 18:55:46, Florian Schneider wrote:
> temp not needed here anymore.

Done.

https://codereview.chromium.org/252333002/diff/220001/runtime/vm/locations.cc
File runtime/vm/locations.cc (right):

https://codereview.chromium.org/252333002/diff/220001/runtime/vm/locations.cc...
runtime/vm/locations.cc:247: return Location::Pair(pair->At(0), pair->At(1));
On 2014/04/30 18:55:46, Florian Schneider wrote:
> I think here you should do a deep copy of the parts as well. Otherwise there
> will be unintended sharing of the location parts.

Done.

[Cutch, 2014-05-14 17:16:31]

+zra for ARM changes.

[zra, 2014-05-14 18:27:41]

https://codereview.chromium.org/252333002/diff/260001/runtime/vm/assembler_ar...
File runtime/vm/assembler_arm.cc (right):

https://codereview.chromium.org/252333002/diff/260001/runtime/vm/assembler_ar...
runtime/vm/assembler_arm.cc:282: void Assembler::SignFill(Register rd, Register
rm) {
Move to be with other macros.

https://codereview.chromium.org/252333002/diff/260001/runtime/vm/assembler_arm.h
File runtime/vm/assembler_arm.h (right):

https://codereview.chromium.org/252333002/diff/260001/runtime/vm/assembler_ar...
runtime/vm/assembler_arm.h:357: void SignFill(Register rd, Register rm);
Move down with other macros.

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

https://codereview.chromium.org/252333002/diff/260001/runtime/vm/intermediate...
runtime/vm/intermediate_language_arm.cc:517: Register left1 =
left_pair->At(0).reg();
optional: rename to {left, right}_{lo, hi}.

https://codereview.chromium.org/252333002/diff/260001/runtime/vm/intermediate...
runtime/vm/intermediate_language_arm.cc:529: __ cmp(left2,
ShifterOperand(right2));
Remove branch, instead:
__ cmp(left2, ShifterOperand(right2), NE);

https://codereview.chromium.org/252333002/diff/260001/runtime/vm/intermediate...
runtime/vm/intermediate_language_arm.cc:539: Register left1 =
left_pair->At(0).reg();
optional: rename to {left, right}_{lo, hi}.

https://codereview.chromium.org/252333002/diff/260001/runtime/vm/intermediate...
runtime/vm/intermediate_language_arm.cc:571: __ b(&is_true, hi_true_cond);
Instead, to avoid so many branches:

__ cmp(left2, ShifterOperand(right2));
__ LoadImmediate(out, 0, hi_false_cond);
__ LoadImmediate(out, 1, hi_true_cond);
__ b(&done, NE);

__ cmp(left1, ShifterOperand(right1);
__ LoadImmediate(out, 1);
__ LoadImmediate(out, 0, lo_false_cond);
__ Bind(&done);

https://codereview.chromium.org/252333002/diff/260001/runtime/vm/intermediate...
runtime/vm/intermediate_language_arm.cc:5767: Register value1 =
value_pair->At(0).reg();
optional: lo/hi instead of 1/2 as above

https://codereview.chromium.org/252333002/diff/260001/runtime/vm/intermediate...
runtime/vm/intermediate_language_arm.cc:5776: // Check high word.
Can you rewrite this to mirror the ia32 implementation? There would be fewer
branches. (My implementation was just quick and dirty.)

https://codereview.chromium.org/252333002/diff/260001/runtime/vm/intermediate...
runtime/vm/intermediate_language_arm.cc:5933: // Deopt if shift is larger than
32.
Are there funky things that happen for >= 32 as part of the spec? If not, just
copy hi into low for shr, or low into hi for shl, then finish the shift by
(amount - 32). overflow check in shl case by asr high word of output by 31 and
comparing with high word of input ... I think.

[Cutch, 2014-05-15 18:26:07]

https://codereview.chromium.org/252333002/diff/260001/runtime/vm/assembler_ar...
File runtime/vm/assembler_arm.cc (right):

https://codereview.chromium.org/252333002/diff/260001/runtime/vm/assembler_ar...
runtime/vm/assembler_arm.cc:282: void Assembler::SignFill(Register rd, Register
rm) {
On 2014/05/14 18:27:41, zra wrote:
> Move to be with other macros.

Done.

https://codereview.chromium.org/252333002/diff/260001/runtime/vm/assembler_arm.h
File runtime/vm/assembler_arm.h (right):

https://codereview.chromium.org/252333002/diff/260001/runtime/vm/assembler_ar...
runtime/vm/assembler_arm.h:357: void SignFill(Register rd, Register rm);
On 2014/05/14 18:27:41, zra wrote:
> Move down with other macros.

Done.

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

https://codereview.chromium.org/252333002/diff/260001/runtime/vm/intermediate...
runtime/vm/intermediate_language_arm.cc:517: Register left1 =
left_pair->At(0).reg();
On 2014/05/14 18:27:41, zra wrote:
> optional: rename to {left, right}_{lo, hi}.

Done.

https://codereview.chromium.org/252333002/diff/260001/runtime/vm/intermediate...
runtime/vm/intermediate_language_arm.cc:529: __ cmp(left2,
ShifterOperand(right2));
On 2014/05/14 18:27:41, zra wrote:
> Remove branch, instead:
> __ cmp(left2, ShifterOperand(right2), NE);

Done but I think you meant EQ instead of NE.

https://codereview.chromium.org/252333002/diff/260001/runtime/vm/intermediate...
runtime/vm/intermediate_language_arm.cc:539: Register left1 =
left_pair->At(0).reg();
On 2014/05/14 18:27:41, zra wrote:
> optional: rename to {left, right}_{lo, hi}.

Done.

https://codereview.chromium.org/252333002/diff/260001/runtime/vm/intermediate...
runtime/vm/intermediate_language_arm.cc:571: __ b(&is_true, hi_true_cond);
On 2014/05/14 18:27:41, zra wrote:
> Instead, to avoid so many branches:
> 
> __ cmp(left2, ShifterOperand(right2));
> __ LoadImmediate(out, 0, hi_false_cond);
> __ LoadImmediate(out, 1, hi_true_cond);
> __ b(&done, NE);
> 
> __ cmp(left1, ShifterOperand(right1);
> __ LoadImmediate(out, 1);
> __ LoadImmediate(out, 0, lo_false_cond);
> __ Bind(&done);

Done.

https://codereview.chromium.org/252333002/diff/260001/runtime/vm/intermediate...
runtime/vm/intermediate_language_arm.cc:5767: Register value1 =
value_pair->At(0).reg();
On 2014/05/14 18:27:41, zra wrote:
> optional: lo/hi instead of 1/2 as above

Done.

https://codereview.chromium.org/252333002/diff/260001/runtime/vm/intermediate...
runtime/vm/intermediate_language_arm.cc:5776: // Check high word.
On 2014/05/14 18:27:41, zra wrote:
> Can you rewrite this to mirror the ia32 implementation? There would be fewer
> branches. (My implementation was just quick and dirty.)

I took a stab at this but kept getting test failures. A separate CL could clean
up the boxing logic.

https://codereview.chromium.org/252333002/diff/260001/runtime/vm/intermediate...
runtime/vm/intermediate_language_arm.cc:5933: // Deopt if shift is larger than
32.
On 2014/05/14 18:27:41, zra wrote:
> Are there funky things that happen for >= 32 as part of the spec? If not, just
> copy hi into low for shr, or low into hi for shl, then finish the shift by
> (amount - 32). overflow check in shl case by asr high word of output by 31 and
> comparing with high word of input ... I think.

Done.

[zra, 2014-05-15 18:30:45]

arm code lgtm

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

https://codereview.chromium.org/252333002/diff/260001/runtime/vm/intermediate...
runtime/vm/intermediate_language_arm.cc:529: __ cmp(left2,
ShifterOperand(right2));
On 2014/05/15 18:26:08, Cutch wrote:
> On 2014/05/14 18:27:41, zra wrote:
> > Remove branch, instead:
> > __ cmp(left2, ShifterOperand(right2), NE);
> 
> Done but I think you meant EQ instead of NE.

Yes, you're right.

https://codereview.chromium.org/252333002/diff/260001/runtime/vm/intermediate...
runtime/vm/intermediate_language_arm.cc:5776: // Check high word.
On 2014/05/15 18:26:08, Cutch wrote:
> On 2014/05/14 18:27:41, zra wrote:
> > Can you rewrite this to mirror the ia32 implementation? There would be fewer
> > branches. (My implementation was just quick and dirty.)
> 
> I took a stab at this but kept getting test failures. A separate CL could
clean
> up the boxing logic.

sgtm

[Florian Schneider, 2014-05-21 12:50:59]

Lgtm

https://codereview.chromium.org/252333002/diff/340001/runtime/vm/deopt_instru...
File runtime/vm/deopt_instructions.cc (right):

https://codereview.chromium.org/252333002/diff/340001/runtime/vm/deopt_instru...
runtime/vm/deopt_instructions.cc:724: static int64_t CombineLoAndHi(int32_t* lo,
int32_t* hi) {
Passing int32_t* suggests that lo and hi are input/output parameters. Since they
are only inputs, I'd make lo and hi int32_t  and just pass the values.

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

https://codereview.chromium.org/252333002/diff/340001/runtime/vm/intermediate...
runtime/vm/intermediate_language_ia32.cc:1026:
Location::RegisterLocation(EDX)));
For the case of Uint32Array you don't actually need fixed registers.

https://codereview.chromium.org/252333002/diff/340001/runtime/vm/intermediate...
runtime/vm/intermediate_language_ia32.cc:1088: ASSERT(result2 == EDX);
Move these two ASSERTs into the Int32Array case (where you need it because of
using cdq)

https://codereview.chromium.org/252333002/diff/340001/runtime/vm/intermediate...
runtime/vm/intermediate_language_ia32.cc:5678: ASSERT(value_lo == EAX);
Where is it assumed that value_lo must be EAX? It seems that you can change it
to an arbitrary register.

https://codereview.chromium.org/252333002/diff/340001/runtime/vm/intermediate...
runtime/vm/intermediate_language_ia32.cc:5679: ASSERT(value_hi == EDX);
Same for value_hi.

[Cutch, 2014-05-21 17:00:03]

https://codereview.chromium.org/252333002/diff/220001/runtime/vm/deopt_instru...
File runtime/vm/deopt_instructions.cc (right):

https://codereview.chromium.org/252333002/diff/220001/runtime/vm/deopt_instru...
runtime/vm/deopt_instructions.cc:724: static int64_t CombineLoAndHi(int32_t* lo,
int32_t* hi) {
On 2014/04/30 18:46:01, srdjan wrote:
> Use utils.h LowHighTo64Bits

Done.

https://codereview.chromium.org/252333002/diff/220001/runtime/vm/deopt_instru...
runtime/vm/deopt_instructions.cc:737:
hi_reg_(static_cast<Register>(hi_reg_as_int)) {}
On 2014/04/30 18:46:01, srdjan wrote:
> Assert int values are valid registers

I have added asserts to the DeoptContext so that any attempt to use a CPU or FPU
register outside of the range will be caught.

https://codereview.chromium.org/252333002/diff/220001/runtime/vm/deopt_instru...
runtime/vm/deopt_instructions.cc:859: flip_(flip) {
On 2014/04/30 18:46:01, srdjan wrote:
> ASSERT reg_as_int is a valid value.

As above, all accesses to registers in the deopt context now have assertions
checking the bounds.

https://codereview.chromium.org/252333002/diff/220001/runtime/vm/deopt_instru...
runtime/vm/deopt_instructions.cc:1513: // There are four cases to consider here:
On 2014/04/30 18:46:01, srdjan wrote:
> Say R: Register, S: Stack-slot

Done.

https://codereview.chromium.org/252333002/diff/340001/runtime/vm/deopt_instru...
File runtime/vm/deopt_instructions.cc (right):

https://codereview.chromium.org/252333002/diff/340001/runtime/vm/deopt_instru...
runtime/vm/deopt_instructions.cc:724: static int64_t CombineLoAndHi(int32_t* lo,
int32_t* hi) {
On 2014/05/21 12:50:59, Florian Schneider wrote:
> Passing int32_t* suggests that lo and hi are input/output parameters. Since
they
> are only inputs, I'd make lo and hi int32_t  and just pass the values.

Done because I switched to a pre-existing function in utils.

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

https://codereview.chromium.org/252333002/diff/340001/runtime/vm/intermediate...
runtime/vm/intermediate_language_ia32.cc:1026:
Location::RegisterLocation(EDX)));
On 2014/05/21 12:50:59, Florian Schneider wrote:
> For the case of Uint32Array you don't actually need fixed registers.

Done.

https://codereview.chromium.org/252333002/diff/340001/runtime/vm/intermediate...
runtime/vm/intermediate_language_ia32.cc:1088: ASSERT(result2 == EDX);
On 2014/05/21 12:50:59, Florian Schneider wrote:
> Move these two ASSERTs into the Int32Array case (where you need it because of
> using cdq)

Done.

https://codereview.chromium.org/252333002/diff/340001/runtime/vm/intermediate...
runtime/vm/intermediate_language_ia32.cc:5678: ASSERT(value_lo == EAX);
On 2014/05/21 12:50:59, Florian Schneider wrote:
> Where is it assumed that value_lo must be EAX? It seems that you can change it
> to an arbitrary register.

Done.

https://codereview.chromium.org/252333002/diff/340001/runtime/vm/intermediate...
runtime/vm/intermediate_language_ia32.cc:5679: ASSERT(value_hi == EDX);
On 2014/05/21 12:50:59, Florian Schneider wrote:
> Same for value_hi.

Done.

[srdjan, 2014-05-21 17:12:48]

lgtm

https://codereview.chromium.org/252333002/diff/360001/runtime/vm/assembler_arm.h
File runtime/vm/assembler_arm.h (right):

https://codereview.chromium.org/252333002/diff/360001/runtime/vm/assembler_ar...
runtime/vm/assembler_arm.h:354: void adcs(Register rd, Register rn,
ShifterOperand so, Condition cond = AL);
Please add assembler testes for the new assembly instructions (2).

https://codereview.chromium.org/252333002/diff/360001/runtime/vm/deopt_instru...
File runtime/vm/deopt_instructions.h (right):

https://codereview.chromium.org/252333002/diff/360001/runtime/vm/deopt_instru...
runtime/vm/deopt_instructions.h:62: ASSERT(reg < kNumberOfCpuRegisters);
I am OK merging these two ASSERTs into one (also for code below). Seems easier
to read as they are tied together, IMO.

https://codereview.chromium.org/252333002/diff/360001/runtime/vm/flow_graph_a...
File runtime/vm/flow_graph_allocator.h (right):

https://codereview.chromium.org/252333002/diff/360001/runtime/vm/flow_graph_a...
runtime/vm/flow_graph_allocator.h:322: // This is used to keep tagged and
untagged spill slots disjoint.
Add a comment why we keep them disjoint

[Cutch, 2014-05-22 06:08:58]

https://codereview.chromium.org/252333002/diff/360001/runtime/vm/assembler_arm.h
File runtime/vm/assembler_arm.h (right):

https://codereview.chromium.org/252333002/diff/360001/runtime/vm/assembler_ar...
runtime/vm/assembler_arm.h:354: void adcs(Register rd, Register rn,
ShifterOperand so, Condition cond = AL);
On 2014/05/21 17:12:49, srdjan wrote:
> Please add assembler testes for the new assembly instructions (2).

Done.

https://codereview.chromium.org/252333002/diff/360001/runtime/vm/flow_graph_a...
File runtime/vm/flow_graph_allocator.h (right):

https://codereview.chromium.org/252333002/diff/360001/runtime/vm/flow_graph_a...
runtime/vm/flow_graph_allocator.h:322: // This is used to keep tagged and
untagged spill slots disjoint.
On 2014/05/21 17:12:49, srdjan wrote:
> Add a comment why we keep them disjoint

Done.

[Cutch, 2014-05-22 06:31:01]

Committed patchset #20 manually as r36468 (presubmit successful).

[regis, 2014-05-22 17:09:40]

DBC

https://codereview.chromium.org/252333002/diff/380001/runtime/vm/assembler_arm.h
File runtime/vm/assembler_arm.h (right):

https://codereview.chromium.org/252333002/diff/380001/runtime/vm/assembler_ar...
runtime/vm/assembler_arm.h:702: void SignFill(Register rd, Register rm);
For regularity, you could make this conditional.

https://codereview.chromium.org/252333002/diff/380001/runtime/vm/deopt_instru...
File runtime/vm/deopt_instructions.h (right):

https://codereview.chromium.org/252333002/diff/380001/runtime/vm/deopt_instru...
runtime/vm/deopt_instructions.h:227: kInt64RegisterPair,
Why not kRegisterPair?
kInt64RegisterPair sounds like a pair of 64-bit registers.

https://codereview.chromium.org/252333002/diff/380001/runtime/vm/deopt_instru...
runtime/vm/deopt_instructions.h:238: kInt64StackSlotRegister,
Comments would help here. What is the difference between a kInt64StackSlot, a
kInt64StackSlotPair, and a kInt64StackSlotRegister? I hope a kInt64StackSlotPair
is twice the size of a kInt64StackSlot, is it?

https://codereview.chromium.org/252333002/diff/380001/runtime/vm/flow_graph_c...
File runtime/vm/flow_graph_compiler_arm.cc (right):

https://codereview.chromium.org/252333002/diff/380001/runtime/vm/flow_graph_c...
runtime/vm/flow_graph_compiler_arm.cc:1455: }
Is there a good reason for this change? It should be documented.

[Cutch, 2014-05-23 21:33:53]

https://codereview.chromium.org/252333002/diff/380001/runtime/vm/deopt_instru...
File runtime/vm/deopt_instructions.h (right):

https://codereview.chromium.org/252333002/diff/380001/runtime/vm/deopt_instru...
runtime/vm/deopt_instructions.h:227: kInt64RegisterPair,
On 2014/05/22 17:09:40, regis wrote:
> Why not kRegisterPair?
> kInt64RegisterPair sounds like a pair of 64-bit registers.

A kInt64RegisterPair is a mint (64-bit integer) which is in two register
locations. 

A kInt64StackSlotPair is a mint which is in two stack slots.

A kInt64StackSlotRegister is a mint which has one half in a register location
and the other half in a stack location.

https://codereview.chromium.org/252333002/diff/380001/runtime/vm/deopt_instru...
runtime/vm/deopt_instructions.h:238: kInt64StackSlotRegister,
On 2014/05/22 17:09:40, regis wrote:
> Comments would help here. What is the difference between a kInt64StackSlot, a
> kInt64StackSlotPair, and a kInt64StackSlotRegister? I hope a
kInt64StackSlotPair
> is twice the size of a kInt64StackSlot, is it?

See above.

https://codereview.chromium.org/252333002/diff/380001/runtime/vm/flow_graph_c...
File runtime/vm/flow_graph_compiler_arm.cc (right):

https://codereview.chromium.org/252333002/diff/380001/runtime/vm/flow_graph_c...
runtime/vm/flow_graph_compiler_arm.cc:1455: }
On 2014/05/22 17:09:40, regis wrote:
> Is there a good reason for this change? It should be documented.

Yes, there was a very serious bug where the stackmap for a safepoint encoded
registers in the *opposite* order than __ PushList pushed them. This resulted in
random crashes during GC where a stack slot was expected to have an object (or
smi) but it had an untagged value. I've added a note in my cleanup CL:

https://codereview.chromium.org/299833011

[regis, 2014-05-23 22:36:58]

https://codereview.chromium.org/252333002/diff/380001/runtime/vm/deopt_instru...
File runtime/vm/deopt_instructions.h (right):

https://codereview.chromium.org/252333002/diff/380001/runtime/vm/deopt_instru...
runtime/vm/deopt_instructions.h:227: kInt64RegisterPair,
On 2014/05/23 21:33:54, Cutch wrote:
> On 2014/05/22 17:09:40, regis wrote:
> > Why not kRegisterPair?
> > kInt64RegisterPair sounds like a pair of 64-bit registers.
> 
> A kInt64RegisterPair is a mint (64-bit integer) which is in two register
> locations. 
> 
> A kInt64StackSlotPair is a mint which is in two stack slots.
> 
> A kInt64StackSlotRegister is a mint which has one half in a register location
> and the other half in a stack location.

Do I understand correctly that some of these are for 32-bit architectures only
and others for 64-bit architectures only?

Marking these as such would avoid confusion:
  kInt64RegisterPair,  // On 32-bit architectures only.
  kInt64StackSlot,  // On 64-bit architectures only.
  kInt64StackSlotPair,  // On 32-bit architectures only.
etc...

Maybe I still do not get it.
Let's chat next week.

https://codereview.chromium.org/252333002/diff/380001/runtime/vm/flow_graph_c...
File runtime/vm/flow_graph_compiler_arm.cc (right):

https://codereview.chromium.org/252333002/diff/380001/runtime/vm/flow_graph_c...
runtime/vm/flow_graph_compiler_arm.cc:1455: }
On 2014/05/23 21:33:54, Cutch wrote:
> On 2014/05/22 17:09:40, regis wrote:
> > Is there a good reason for this change? It should be documented.
> 
> Yes, there was a very serious bug where the stackmap for a safepoint encoded
> registers in the *opposite* order than __ PushList pushed them. This resulted
in
> random crashes during GC where a stack slot was expected to have an object (or
> smi) but it had an untagged value. I've added a note in my cleanup CL:
> 
> https://codereview.chromium.org/299833011

Cool you found it!