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

Issue 2179503003: PPC/s390: [turbofan] Change Float64Max/Float64Min to JavaScript semantics. (Closed)

Created:
4 years, 5 months ago by JaideepBajwa
Modified:
4 years, 5 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/s390: [turbofan] Change Float64Max/Float64Min to JavaScript semantics. Port ba092fb09abb6ba3b7154444510c3b4cbdcf65f5 Original commit message: So far we don't have a useful way to inline Math.max or Math.min in TurboFan optimized code. This adds new operators NumberMax and NumberMin and changes the Float64Max/Float64Min operators to have JavaScript semantics instead of the C++ semantics that it had previously. This also removes support for recognizing the tenary case in the CommonOperatorReducer, since that doesn't seem to have any positive impact (and actually doesn't show up in regular JavaScript, where people use Math.max/Math.min instead). Drive-by-fix: Also nuke the unused Float32Max/Float32Min operators. R=bmeurer@chromium.org, joransiu@ca.ibm.com, jyan@ca.ibm.com, michael_dawson@ca.ibm.com, mbrandy@us.ibm.com BUG= LOG=N Committed: https://crrev.com/aed69fd919dac8caa76bafc95bdcb9402a890014 Cr-Commit-Position: refs/heads/master@{#37991}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+185 lines, -51 lines) Patch
M src/compiler/ppc/code-generator-ppc.cc View 2 chunks +83 lines, -14 lines 0 comments Download
M src/compiler/ppc/instruction-selector-ppc.cc View 1 chunk +6 lines, -7 lines 0 comments Download
M src/compiler/s390/code-generator-s390.cc View 2 chunks +90 lines, -24 lines 0 comments Download
M src/compiler/s390/instruction-selector-s390.cc View 1 chunk +6 lines, -6 lines 0 comments Download

Messages

Total messages: 7 (2 generated)
JaideepBajwa
PTAL
4 years, 5 months ago (2016-07-23 00:35:54 UTC) #1
john.yan
lgtm
4 years, 5 months ago (2016-07-23 00:38:19 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2179503003/1
4 years, 5 months ago (2016-07-23 00:46:07 UTC) #4
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 5 months ago (2016-07-23 01:09:12 UTC) #5
commit-bot: I haz the power
4 years, 5 months ago (2016-07-23 01:10:25 UTC) #7
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/aed69fd919dac8caa76bafc95bdcb9402a890014
Cr-Commit-Position: refs/heads/master@{#37991}

Powered by Google App Engine
This is Rietveld 408576698