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

Issue 2462173002: PPC/s390: [turbofan] Support variable size argument removal in TF-generated functions (Closed)

Created:
4 years, 1 month ago by JaideepBajwa
Modified:
4 years, 1 month ago
CC:
v8-reviews_googlegroups.com
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

PPC/s390: [turbofan] Support variable size argument removal in TF-generated functions Port 5319b50c853c4213d825aa7cf620fde5d827f7eb Original commit message: This is preparation for using TF to create builtins that handle variable number of arguments and have to remove these arguments dynamically from the stack upon return. The gist of the changes: - Added a second argument to the Return node which specifies the number of stack slots to pop upon return in addition to those specified by the Linkage of the compiled function. - Removed Tail -> Non-Tail fallback in the instruction selector. Since TF now should handles all tail-call cases except where the return value type differs, this fallback was not really useful and in fact caused unexpected behavior with variable sized argument popping, since it wasn't possible to materialize a Return node with the right pop count from the TailCall without additional context. - Modified existing Return generation to pass a constant zero as the additional pop argument since the variable pop functionality R=danno@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/2c846a2ac7dd93f18f453a1178abca414b4155cf Cr-Commit-Position: refs/heads/master@{#40682}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+42 lines, -17 lines) Patch
M src/compiler/ppc/code-generator-ppc.cc View 3 chunks +21 lines, -9 lines 0 comments Download
M src/compiler/s390/code-generator-s390.cc View 3 chunks +21 lines, -8 lines 0 comments Download

Messages

Total messages: 7 (2 generated)
JaideepBajwa
PTAL
4 years, 1 month ago (2016-10-31 19:30:36 UTC) #1
john.yan
lgtm
4 years, 1 month ago (2016-10-31 19:32:06 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/2462173002/1
4 years, 1 month ago (2016-10-31 19:32:44 UTC) #4
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 1 month ago (2016-10-31 20:00:02 UTC) #5
commit-bot: I haz the power
4 years, 1 month ago (2016-11-17 22:18:35 UTC) #7
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/2c846a2ac7dd93f18f453a1178abca414b4155cf
Cr-Commit-Position: refs/heads/master@{#40682}

Powered by Google App Engine
This is Rietveld 408576698