ARM: Let the register allocator handle the context register.

ARM: Let the register allocator handle the context register.

BUG=none
TEST=none
R=ulan@chromium.org

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

[Rodolph Perfetta, 2013-09-25 15:20:36]

A further patch will enable inlining of methods with different contexts.

[ulan, 2013-09-26 10:06:33]

Looks good. First round of comments.

https://codereview.chromium.org/24596002/diff/1/src/arm/lithium-codegen-arm.cc
File src/arm/lithium-codegen-arm.cc (right):

https://codereview.chromium.org/24596002/diff/1/src/arm/lithium-codegen-arm.c...
src/arm/lithium-codegen-arm.cc:2587: void
LCodeGen::DoStringCompareAndBranch(LStringCompareAndBranch* instr) {
ASSERT(ToRegister(instr->context()).is(cp));

https://codereview.chromium.org/24596002/diff/1/src/arm/lithium-codegen-arm.c...
src/arm/lithium-codegen-arm.cc:2902: void LCodeGen::DoCmpT(LCmpT* instr) {
ASSERT(ToRegister(instr->context()).is(cp));

https://codereview.chromium.org/24596002/diff/1/src/arm/lithium-codegen-arm.c...
src/arm/lithium-codegen-arm.cc:3996:
ASSERT(ToDoubleRegister(instr->result()).is(d2));
Set cp as in other DoMath* functions.

https://codereview.chromium.org/24596002/diff/1/src/arm/lithium-codegen-arm.c...
src/arm/lithium-codegen-arm.cc:4565: void LCodeGen::DoStringAdd(LStringAdd*
instr) {
ASSERT(ToRegister(instr->context()).is(cp));

https://codereview.chromium.org/24596002/diff/1/src/arm/lithium-codegen-arm.c...
src/arm/lithium-codegen-arm.cc:5726:
ASSERT(instr->hydrogen()->is_backwards_branch());
Indentation is off.

[Rodolph Perfetta, 2013-09-26 12:20:12]

https://codereview.chromium.org/24596002/diff/1/src/arm/lithium-codegen-arm.cc
File src/arm/lithium-codegen-arm.cc (right):

https://codereview.chromium.org/24596002/diff/1/src/arm/lithium-codegen-arm.c...
src/arm/lithium-codegen-arm.cc:2587: void
LCodeGen::DoStringCompareAndBranch(LStringCompareAndBranch* instr) {
On 2013/09/26 10:06:33, ulan wrote:
> ASSERT(ToRegister(instr->context()).is(cp));

Done.

https://codereview.chromium.org/24596002/diff/1/src/arm/lithium-codegen-arm.c...
src/arm/lithium-codegen-arm.cc:2902: void LCodeGen::DoCmpT(LCmpT* instr) {
On 2013/09/26 10:06:33, ulan wrote:
> ASSERT(ToRegister(instr->context()).is(cp));

Done.

https://codereview.chromium.org/24596002/diff/1/src/arm/lithium-codegen-arm.c...
src/arm/lithium-codegen-arm.cc:3996:
ASSERT(ToDoubleRegister(instr->result()).is(d2));
On 2013/09/26 10:06:33, ulan wrote:
> Set cp as in other DoMath* functions.

Done.

https://codereview.chromium.org/24596002/diff/1/src/arm/lithium-codegen-arm.c...
src/arm/lithium-codegen-arm.cc:4565: void LCodeGen::DoStringAdd(LStringAdd*
instr) {
On 2013/09/26 10:06:33, ulan wrote:
> ASSERT(ToRegister(instr->context()).is(cp));

Done.

https://codereview.chromium.org/24596002/diff/1/src/arm/lithium-codegen-arm.c...
src/arm/lithium-codegen-arm.cc:5726:
ASSERT(instr->hydrogen()->is_backwards_branch());
On 2013/09/26 10:06:33, ulan wrote:
> Indentation is off.

Done.

[ulan, 2013-09-27 12:08:00]

LGTM

https://codereview.chromium.org/24596002/diff/11001/src/arm/lithium-arm.cc
File src/arm/lithium-arm.cc (right):

https://codereview.chromium.org/24596002/diff/11001/src/arm/lithium-arm.cc#ne...
src/arm/lithium-arm.cc:1214: : UseFixed(instr->context(), cp);
I think we can use UseAny here because the context is used in deferred code.

https://codereview.chromium.org/24596002/diff/11001/src/arm/lithium-codegen-a...
File src/arm/lithium-codegen-arm.cc (right):

https://codereview.chromium.org/24596002/diff/11001/src/arm/lithium-codegen-a...
src/arm/lithium-codegen-arm.cc:3683:
ASSERT(ToRegister(instr->context()).is(cp));
I think we don't need context in cp here since it is loaded in call to runtime
from deferred.

[vincent.belliard.fr, 2013-09-27 12:37:04]

https://codereview.chromium.org/24596002/diff/11001/src/arm/lithium-arm.cc
File src/arm/lithium-arm.cc (right):

https://codereview.chromium.org/24596002/diff/11001/src/arm/lithium-arm.cc#ne...
src/arm/lithium-arm.cc:1214: : UseFixed(instr->context(), cp);
On 2013/09/27 12:08:00, ulan wrote:
> I think we can use UseAny here because the context is used in deferred code.

If we use UseAny, the context will we copied into cp if necessary. However, as
at the end we need the context in cp, I think it's better to let the allocator
moving the context into cp (in some case, it should avoid some mov).

In fact, it's just a question of probability, we should use the deferred code
most of the time because it's not used only when we have a tagged value which is
a smi (untagged cases don't need the context). I think that there are more
probabilities that a tagged value hold a number or an integer than a smi because
the cases untagged number, untagged integer or smi are already managed by a
different path.

If you think these assertions are false, I will put UseAny.

https://codereview.chromium.org/24596002/diff/11001/src/arm/lithium-codegen-a...
File src/arm/lithium-codegen-arm.cc (right):

https://codereview.chromium.org/24596002/diff/11001/src/arm/lithium-codegen-a...
src/arm/lithium-codegen-arm.cc:3683:
ASSERT(ToRegister(instr->context()).is(cp));
On 2013/09/27 12:08:00, ulan wrote:
> I think we don't need context in cp here since it is loaded in call to runtime
> from deferred.

It is just to be consistent with LChunkBuilder::DoMathAbs. If we use UseAny,
these assert will be removed.

[ulan, 2013-09-27 12:44:13]

https://codereview.chromium.org/24596002/diff/11001/src/arm/lithium-arm.cc
File src/arm/lithium-arm.cc (right):

https://codereview.chromium.org/24596002/diff/11001/src/arm/lithium-arm.cc#ne...
src/arm/lithium-arm.cc:1214: : UseFixed(instr->context(), cp);
OK, let's leave UseFixed.

[Rodolph Perfetta, 2013-09-27 13:59:43]

Committed patchset #2 manually as r16993 (presubmit successful).