Merge Label and NearLabel

Merge Label and NearLabel

by adding NearLabel's functionality to Label and introducing a "near" parameter to jump instructions.


TEST=compiles; existing tests still pass.

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

[Jakob Kummerow, 2011-05-06 14:06:28]

Reviewer's guide:

This CL contains two types of changes:
1.) The actual change, i.e. adding NearLabel's functionality to Label, and the
required updates to the bind/jump instructions. These changes are in:
assembler.h
ia32/assembler-ia32{.h, -inl.h, .cc}
x64/assembler-x64{.h, .cc}

2.) A bunch of purely mechanical refactorings to make use of these changes
(otherwise there'd be no way to test them). These are in all of the other
changed files. You'll see mainly two patterns:
a) Changing a NearLabel/jump pair:
NearLabel xyz;
__ jmp(&xyz);
to the new way to do it:
Label xyz;
__ jmp(&xyz, Label::kNear);
b) Adding an explicit "near" parameter to jumps specifying a branch hint (since
both parameters are optional and therefore only distinguished by position).
Example:
__ jcc(cc, &xyz, hint);
needs to be this from now on:
__ jcc(cc, &xyz, Label::kFar, hint);

I've left out all non-trivial refactorings in order to not make this CL any
larger than it already is.


There are two obvious follow-up CLs left to be done:
• Refactor the remaining uses of NearLabel, and remove that class completely.
• Find all long jumps that could be replaced by short jumps (by specifying the
Label::kNear parameter).


Please take a look.

[Mads Ager (chromium), 2011-05-09 11:21:55]

LGTM

http://codereview.chromium.org/6928060/diff/1/src/assembler.h
File src/assembler.h (right):

http://codereview.chromium.org/6928060/diff/1/src/assembler.h#newcode81
src/assembler.h:81: static const bool kNear = true, kFar = false;
Make this an enum instead.

[danno, 2011-05-09 14:59:56]

LGTM, if you fix my comment :-)

http://codereview.chromium.org/6928060/diff/1/src/ia32/assembler-ia32.cc
File src/ia32/assembler-ia32.cc (right):

http://codereview.chromium.org/6928060/diff/1/src/ia32/assembler-ia32.cc#newc...
src/ia32/assembler-ia32.cc:1704: void Assembler::j(Condition cc, Label* L, bool
near, Hint hint) {
Can you please make this the last optional parameter? Now that you've found and
replaced all of them, that shouldn't be so hard. Makes the far (default) case
compacter.

[fschneider, 2011-05-09 15:37:36]

http://codereview.chromium.org/6928060/diff/1/src/ia32/assembler-ia32.cc
File src/ia32/assembler-ia32.cc (right):

http://codereview.chromium.org/6928060/diff/1/src/ia32/assembler-ia32.cc#newc...
src/ia32/assembler-ia32.cc:1704: void Assembler::j(Condition cc, Label* L, bool
near, Hint hint) {
On 2011/05/09 14:59:56, danno wrote:
> Can you please make this the last optional parameter? Now that you've found
and
> replaced all of them, that shouldn't be so hard. Makes the far (default) case
> compacter.

I think we could even get rid of the Hint parameter. No modern Intel
architecture actually benefits from these anyway. I'm not sure if we emit them
at all in the generated code.

[Jakob Kummerow, 2011-05-09 16:25:23]

Combining your ideas made this CL a good deal less verbose :-)

I'll land it tomorrow morning, unless you have more comments.

http://codereview.chromium.org/6928060/diff/1/src/assembler.h
File src/assembler.h (right):

http://codereview.chromium.org/6928060/diff/1/src/assembler.h#newcode81
src/assembler.h:81: static const bool kNear = true, kFar = false;
On 2011/05/09 11:21:55, Mads Ager wrote:
> Make this an enum instead.

Done.

http://codereview.chromium.org/6928060/diff/1/src/ia32/assembler-ia32.cc
File src/ia32/assembler-ia32.cc (right):

http://codereview.chromium.org/6928060/diff/1/src/ia32/assembler-ia32.cc#newc...
src/ia32/assembler-ia32.cc:1704: void Assembler::j(Condition cc, Label* L, bool
near, Hint hint) {
On 2011/05/09 14:59:56, danno wrote:
> Can you please make this the last optional parameter? Now that you've found
and
> replaced all of them, that shouldn't be so hard. Makes the far (default) case
> compacter.

Done.

http://codereview.chromium.org/6928060/diff/1/src/ia32/assembler-ia32.cc#newc...
src/ia32/assembler-ia32.cc:1704: void Assembler::j(Condition cc, Label* L, bool
near, Hint hint) {
On 2011/05/09 15:37:36, fschneider wrote:
> On 2011/05/09 14:59:56, danno wrote:
> > Can you please make this the last optional parameter? Now that you've found
> and
> > replaced all of them, that shouldn't be so hard. Makes the far (default)
case
> > compacter.
> 
> I think we could even get rid of the Hint parameter. No modern Intel
> architecture actually benefits from these anyway. I'm not sure if we emit them
> at all in the generated code.

shell --help says:
  --emit_branch_hints (emit branch hints)
        type: bool  default: false
So yeah, I guess we can get rid of them. But I would propose to do that in
another CL, since it's not directly related to the NearLabel refactoring.

[danno, 2011-05-10 08:39:08]

still LGTM