Add inline non-transcendental cache version of log to lithium.

Add inline non-transcendental cache version of log to lithium.

In addition, this change allows one additional level of inlining. 
Committed: http://code.google.com/p/v8/source/detail?r=7500

[fschneider, 2011-04-04 13:05:54]

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

http://codereview.chromium.org/6720017/diff/1/src/ia32/lithium-codegen-ia32.c...
src/ia32/lithium-codegen-ia32.cc:2780: XMMRegister input_reg =
ToDoubleRegister(instr->InputAt(0));
Drive-by: for optimizal performance make sure to also update the register
constraints in  LChunkBuilder::DoUnaryMathOperation. e.g. it seems MathLog does
not need MarkAsCall and fixed input/output registers anymore.

[Kevin Millikin (Chromium), 2011-04-04 13:17:10]

Florian's right, you should update the Hydrogen->Lithium translation.  I'd like
to see that before giving an LGTM.

You could also ask Bill or somebody to take a quick look at the fp code.  It
seems OK to me, but I might have missed something.

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

http://codereview.chromium.org/6720017/diff/1/src/hydrogen.cc#newcode3922
src/hydrogen.cc:3922: // Don't inline deeper than tree calls.
tree --> three

[Rico, 2011-04-04 13:26:04]

Bill, could you take a look at fpu code?

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

http://codereview.chromium.org/6720017/diff/1/src/hydrogen.cc#newcode3922
src/hydrogen.cc:3922: // Don't inline deeper than tree calls.
On 2011/04/04 13:17:10, Kevin Millikin wrote:
> tree --> three

Done.

[Lasse Reichstein, 2011-04-04 13:38:22]

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

http://codereview.chromium.org/6720017/diff/1/src/ia32/lithium-codegen-ia32.c...
src/ia32/lithium-codegen-ia32.cc:2792: __ movdbl(input_reg, Operand(esp, 0));
You can load -Infinity into an XMM register without using memory by loading it
as float and converting to double (if you have a temp register).
I.e., something like
  mov(temp, Immediate(kFloatNegInfinityBits)); // whatever the bits are
  movd(input_reg, temp);  
  cvtss2sd(input_reg, input_reg);

(If you don't have a temp register, the code is fine. It's not like it's the
common path for real code :).

[Søren Thygesen Gjesse, 2011-04-05 06:39:07]

On 2011/04/04 13:38:22, Lasse Reichstein wrote:
> http://codereview.chromium.org/6720017/diff/1/src/ia32/lithium-codegen-ia32.cc
> File src/ia32/lithium-codegen-ia32.cc (right):
> 
>
http://codereview.chromium.org/6720017/diff/1/src/ia32/lithium-codegen-ia32.c...
> src/ia32/lithium-codegen-ia32.cc:2792: __ movdbl(input_reg, Operand(esp, 0));
> You can load -Infinity into an XMM register without using memory by loading it
> as float and converting to double (if you have a temp register).
> I.e., something like
>   mov(temp, Immediate(kFloatNegInfinityBits)); // whatever the bits are
>   movd(input_reg, temp);  
>   cvtss2sd(input_reg, input_reg);
> 
> (If you don't have a temp register, the code is fine. It's not like it's the
> common path for real code :).

In both top.cc and runtime.cc we have the following code

    List<FrameSummary> frames(3);  // Max 2 levels of inlining.
    frame->Summarize(&frames);

I think we should have some constant for the inlining level to be in general to
avoid code like this suddenly causing crashes. There might be other places with
hardcoded 2 or 3 (I should think the constant 3 here is still safe - although it
does not match the comment).

[Rico, 2011-04-05 07:12:45]

Comments addressed, added Compiler::kMaxInliningLevels and change the code to
not mark as call for log.

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

http://codereview.chromium.org/6720017/diff/1/src/ia32/lithium-codegen-ia32.c...
src/ia32/lithium-codegen-ia32.cc:2780: XMMRegister input_reg =
ToDoubleRegister(instr->InputAt(0));
On 2011/04/04 13:05:54, fschneider wrote:
> Drive-by: for optimizal performance make sure to also update the register
> constraints in  LChunkBuilder::DoUnaryMathOperation. e.g. it seems MathLog
does
> not need MarkAsCall and fixed input/output registers anymore.

Done.

http://codereview.chromium.org/6720017/diff/1/src/ia32/lithium-codegen-ia32.c...
src/ia32/lithium-codegen-ia32.cc:2792: __ movdbl(input_reg, Operand(esp, 0));
On 2011/04/04 13:38:23, Lasse Reichstein wrote:
> You can load -Infinity into an XMM register without using memory by loading it
> as float and converting to double (if you have a temp register).
> I.e., something like
>   mov(temp, Immediate(kFloatNegInfinityBits)); // whatever the bits are
>   movd(input_reg, temp);  
>   cvtss2sd(input_reg, input_reg);
> 
> (If you don't have a temp register, the code is fine. It's not like it's the
> common path for real code :).

No temp register

[fschneider, 2011-04-05 07:31:05]

LGTM with one comment.

http://codereview.chromium.org/6720017/diff/1008/src/ia32/lithium-ia32.cc
File src/ia32/lithium-ia32.cc (right):

http://codereview.chromium.org/6720017/diff/1008/src/ia32/lithium-ia32.cc#new...
src/ia32/lithium-ia32.cc:1230: if (op == kMathLog) {
You don't need a fixed register for MathLog either.

I'd rewrite the whole into one if-statement like

if (op == kMathLog) {
  ASSERT(instr->representation().IsDouble());
  ASSERT(instr->value()->representation().IsDouble());
  LOperand* input = UseRegisterAtStart(instr->value());
  LUnaryMathOperation* result =
    new LUnaryMathOperation(input);
  return DefineSameAsFirst(result);
} else if (op == kMathSin || op == kMathCos) {
  // As before...

} else {
  // As before...
}

[Kevin Millikin (Chromium), 2011-04-05 07:33:19]

http://codereview.chromium.org/6720017/diff/1008/src/ia32/lithium-codegen-ia3...
File src/ia32/lithium-codegen-ia32.cc (right):

http://codereview.chromium.org/6720017/diff/1008/src/ia32/lithium-codegen-ia3...
src/ia32/lithium-codegen-ia32.cc:2779:
ASSERT(ToDoubleRegister(instr->result()).is(xmm1));
This doesn't put the result in xmm1, does it?

[fschneider, 2011-04-05 07:36:09]

http://codereview.chromium.org/6720017/diff/1008/src/ia32/lithium-codegen-ia3...
File src/ia32/lithium-codegen-ia32.cc (right):

http://codereview.chromium.org/6720017/diff/1008/src/ia32/lithium-codegen-ia3...
src/ia32/lithium-codegen-ia32.cc:2779:
ASSERT(ToDoubleRegister(instr->result()).is(xmm1));
On 2011/04/05 07:33:20, Kevin Millikin wrote:
> This doesn't put the result in xmm1, does it?

Result + input should be allowed any double register. This assert should go also
away then.

[Rico, 2011-04-05 07:54:54]

Did Florians solution, and do not require the input or result to be in a
specific register.

[fschneider, 2011-04-05 08:53:40]

LGTM.

On 2011/04/05 07:54:54, Rico wrote:
> Did Florians solution, and do not require the input or result to be in a
> specific register.