Crankshaft support for StringCharFromCode.

Crankshaft support for StringCharFromCode.

This allows more efficient implementations of string keyed access,
String.prototype.chatAt, and String.fromCharCode.

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

[Vitaly Repeshko, 2011-03-13 19:11:23]

If the approach looks good, I'll port it to other architectures and remove the
unused stubs that were added to implement the inline runtime functions.

-- Vitaly

[fschneider, 2011-03-14 11:34:17]

ia32 part LGTM.

http://codereview.chromium.org/6682025/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

http://codereview.chromium.org/6682025/diff/1/src/hydrogen.cc#newcode4105
src/hydrogen.cc:4105: }
You could save a line by writing:
if (id == kStringCharCodeAt) {
  ast_context()->ReturnInstruction(char_code, expr->id());
} else {
  ...
}
return true;

http://codereview.chromium.org/6682025/diff/1/src/ia32/lithium-codegen-ia32.cc
File src/ia32/lithium-codegen-ia32.cc (right):

http://codereview.chromium.org/6682025/diff/1/src/ia32/lithium-codegen-ia32.c...
src/ia32/lithium-codegen-ia32.cc:3054: instr->pointer_map(), 1,
Safepoint::kNoDeoptimizationIndex);
I think it is ok to have no deoptimization index here, but you need to be sure
that we never get a deoptimization when calling the runtime. (e.g. when runtime
code calls JS code which can always cause a forced deopt)

Otherwise it is always safe to assign a lazy deoptimization environment.

[Vitaly Repeshko, 2011-03-14 14:15:50]

Florian, thanks for the first review pass. Please take another look now that the
ports are done.


-- Vitaly

http://codereview.chromium.org/6682025/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

http://codereview.chromium.org/6682025/diff/1/src/hydrogen.cc#newcode4105
src/hydrogen.cc:4105: }
On 2011/03/14 11:34:17, fschneider wrote:
> You could save a line by writing:
> if (id == kStringCharCodeAt) {
>   ast_context()->ReturnInstruction(char_code, expr->id());
> } else {
>   ...
> }
> return true;

This would replace "return true" with "} else {". Sorry, I'm a huge fan of early
returns :)

http://codereview.chromium.org/6682025/diff/1/src/ia32/lithium-codegen-ia32.cc
File src/ia32/lithium-codegen-ia32.cc (right):

http://codereview.chromium.org/6682025/diff/1/src/ia32/lithium-codegen-ia32.c...
src/ia32/lithium-codegen-ia32.cc:3054: instr->pointer_map(), 1,
Safepoint::kNoDeoptimizationIndex);
On 2011/03/14 11:34:17, fschneider wrote:
> I think it is ok to have no deoptimization index here, but you need to be sure
> that we never get a deoptimization when calling the runtime. (e.g. when
runtime
> code calls JS code which can always cause a forced deopt)
> 
> Otherwise it is always safe to assign a lazy deoptimization environment.
> 

Thanks for the explanation! But if this runtime functions re-enters JS, then we
have a more severe problem because we declared that StringFromCharCode has no
visible side effects.

[fschneider, 2011-03-14 15:04:07]

LGTM.