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

Issue 1648353002: PPC: [builtins] Make Math.max and Math.min fast by default. (Closed)

Created:
4 years, 10 months ago by MTBrandyberry
Modified:
4 years, 10 months ago
CC:
v8-reviews_googlegroups.com
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

PPC: [builtins] Make Math.max and Math.min fast by default. Port cb9b801069c432e745f0651cf00d65ef88cd7f06 Original commit message: The previous versions of Math.max and Math.min made it difficult to optimize those (that's why we already have custom code in Crankshaft), and due to lack of ideas what to do about the variable number of arguments, we will probably need to stick in special code in TurboFan as well; so inlining those builtins is off the table, hence there's no real advantage in having them around as "not quite JS" with extra work necessary in the optimizing compilers to still make those builtins somewhat fast in cases where we cannot inline them (also there's a tricky deopt loop in Crankshaft related to Math.min and Math.max, but that will be dealt with later). So to sum up: Instead of trying to make Math.max and Math.min semi-fast in the optimizing compilers with weird work-arounds support %_Arguments %_ArgumentsLength, we do provide the optimal code as native builtins instead and call it a day (which gives a nice performance boost on some benchmarks). R=bmeurer@chromium.org, joransiu@ca.ibm.com, jyan@ca.ibm.com, michael_dawson@ca.ibm.com BUG= Committed: https://crrev.com/3641a448836c08b691650eeddade07a5ee417031 Cr-Commit-Position: refs/heads/master@{#33618}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+118 lines, -0 lines) Patch
M src/ppc/builtins-ppc.cc View 1 chunk +113 lines, -0 lines 0 comments Download
M src/ppc/macro-assembler-ppc.h View 1 chunk +1 line, -0 lines 0 comments Download
M src/ppc/macro-assembler-ppc.cc View 1 chunk +4 lines, -0 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 11 (4 generated)
MTBrandyberry
4 years, 10 months ago (2016-01-29 16:20:06 UTC) #1
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1648353002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1648353002/1
4 years, 10 months ago (2016-01-29 16:21:35 UTC) #3
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 10 months ago (2016-01-29 16:43:08 UTC) #5
michael_dawson
On 2016/01/29 16:43:08, commit-bot: I haz the power wrote: > Dry run: This issue passed ...
4 years, 10 months ago (2016-01-29 18:13:59 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1648353002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1648353002/1
4 years, 10 months ago (2016-01-29 19:29:38 UTC) #8
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 10 months ago (2016-01-29 19:32:44 UTC) #9
commit-bot: I haz the power
4 years, 10 months ago (2016-01-29 19:33:14 UTC) #11
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/3641a448836c08b691650eeddade07a5ee417031
Cr-Commit-Position: refs/heads/master@{#33618}

Powered by Google App Engine
This is Rietveld 408576698