Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(166)

Issue 5640004: Allow the optimizing code generator to call Math.pow with untagged doubles. (Closed)

Created:
10 years ago by William Hesse
Modified:
9 years, 7 months ago
CC:
v8-dev
Visibility:
Public.

Description

Allow the optimizing code generator to call Math.pow with untagged doubles. Committed: http://code.google.com/p/v8/source/detail?r=5949

Patch Set 1 #

Patch Set 2 : '' #

Total comments: 16

Patch Set 3 : '' #

Total comments: 15

Patch Set 4 : '' #

Patch Set 5 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+239 lines, -64 lines) Patch
M src/assembler.h View 1 2 4 chunks +8 lines, -0 lines 0 comments Download
M src/assembler.cc View 1 2 3 4 3 chunks +52 lines, -0 lines 0 comments Download
M src/hydrogen.cc View 1 2 2 chunks +39 lines, -3 lines 0 comments Download
M src/hydrogen-instructions.h View 1 2 3 5 chunks +20 lines, -0 lines 0 comments Download
M src/ia32/lithium-codegen-ia32.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M src/ia32/lithium-codegen-ia32.cc View 1 2 3 4 2 chunks +64 lines, -0 lines 0 comments Download
M src/ia32/lithium-ia32.h View 1 2 3 4 3 chunks +12 lines, -0 lines 0 comments Download
M src/ia32/lithium-ia32.cc View 1 2 3 4 2 chunks +18 lines, -0 lines 0 comments Download
M src/math.js View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M src/objects.h View 1 2 2 chunks +7 lines, -5 lines 0 comments Download
M src/runtime.cc View 1 2 3 chunks +5 lines, -56 lines 0 comments Download
M src/serialize.cc View 1 2 3 4 1 chunk +12 lines, -0 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
William Hesse
Additions still needed for compilation on ARM and x64.
10 years ago (2010-12-07 16:45:43 UTC) #1
William Hesse
10 years ago (2010-12-07 21:42:38 UTC) #2
Kevin Millikin (Chromium)
Quick drive by. http://codereview.chromium.org/5640004/diff/10001/src/assembler.cc File src/assembler.cc (right): http://codereview.chromium.org/5640004/diff/10001/src/assembler.cc#newcode881 src/assembler.cc:881: case Token::BIT_XOR: Don't repurpose these tokens ...
10 years ago (2010-12-08 10:01:45 UTC) #3
fschneider
http://codereview.chromium.org/5640004/diff/10001/src/assembler.cc File src/assembler.cc (right): http://codereview.chromium.org/5640004/diff/10001/src/assembler.cc#newcode801 src/assembler.cc:801: It would be good to not duplicate code between ...
10 years ago (2010-12-08 11:27:07 UTC) #4
William Hesse
Suggested changes made. http://codereview.chromium.org/5640004/diff/10001/src/assembler.cc File src/assembler.cc (right): http://codereview.chromium.org/5640004/diff/10001/src/assembler.cc#newcode801 src/assembler.cc:801: On 2010/12/08 11:27:07, fschneider wrote: > ...
10 years ago (2010-12-08 13:37:00 UTC) #5
Florian Schneider
LGTM with comments below. http://codereview.chromium.org/5640004/diff/24001/src/assembler.cc File src/assembler.cc (right): http://codereview.chromium.org/5640004/diff/24001/src/assembler.cc#newcode808 src/assembler.cc:808: double m = (y < ...
10 years ago (2010-12-08 14:09:27 UTC) #6
William Hesse
10 years ago (2010-12-08 14:53:16 UTC) #7
http://codereview.chromium.org/5640004/diff/24001/src/assembler.cc
File src/assembler.cc (right):

http://codereview.chromium.org/5640004/diff/24001/src/assembler.cc#newcode808
src/assembler.cc:808: double m = (y < 0) ? 1/x : x;
On 2010/12/08 14:09:27, fschneider1 wrote:
> --> 1 / x

Done.

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

http://codereview.chromium.org/5640004/diff/24001/src/ia32/lithium-ia32.cc#ne...
src/ia32/lithium-ia32.cc:1578: //    Abort("Tagged expoinent in Math.pow not yet
workign.");
On 2010/12/08 14:09:27, fschneider1 wrote:
> Remove commented code?

Done.

http://codereview.chromium.org/5640004/diff/24001/src/ia32/lithium-ia32.cc#ne...
src/ia32/lithium-ia32.cc:1586: return MarkAsCall(DefineFixedDouble(result,
xmm1), instr);
On 2010/12/08 14:09:27, fschneider1 wrote:
> In case of a tagged exponent you need to pass CAN_DEOPTIMIZE_EAGERLY to
> MarkAsCall.
>  

Done.

http://codereview.chromium.org/5640004/diff/24001/src/ia32/lithium-ia32.h
File src/ia32/lithium-ia32.h (right):

http://codereview.chromium.org/5640004/diff/24001/src/ia32/lithium-ia32.h#new...
src/ia32/lithium-ia32.h:144: //     LUnknownOSRValue
On 2010/12/08 14:09:27, fschneider1 wrote:
> Accidental edit? UnknownOSRValue is not a UnaryOperation.

Done.

http://codereview.chromium.org/5640004/diff/24001/src/ia32/lithium-ia32.h#new...
src/ia32/lithium-ia32.h:233: V(Power)                               \
On 2010/12/08 14:09:27, fschneider1 wrote:
> Align \

Done.

http://codereview.chromium.org/5640004/diff/24001/src/ia32/lithium-ia32.h#new...
src/ia32/lithium-ia32.h:1165: Representation exponent_type() { return
exponent_type_; }
On 2010/12/08 14:09:27, fschneider1 wrote:
> You can save the exponent_type_ member here and get the representation of the
> expononent from the H-instruction via the hydrogen()-accessor 
> 
> return hydrogen()->right()->representation()
> 
> and by declaring: 
> 
> DECLARE_HYDROGEN_ACCESSOR(Power)

Done.

http://codereview.chromium.org/5640004/diff/24001/src/ia32/lithium-ia32.h#new...
src/ia32/lithium-ia32.h:1165: Representation exponent_type() { return
exponent_type_; }
On 2010/12/08 14:09:27, fschneider1 wrote:
> You can save the exponent_type_ member here and get the representation of the
> expononent from the H-instruction via the hydrogen()-accessor 
> 
> return hydrogen()->right()->representation()
> 
> and by declaring: 
> 
> DECLARE_HYDROGEN_ACCESSOR(Power)

Done.

Powered by Google App Engine
This is Rietveld 408576698