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

Issue 1636353002: [x87] Keep x87 FPU stack empty when calling or return to natvie C++ function. (Closed)

Created:
4 years, 11 months ago by zhengxing.li
Modified:
4 years, 10 months ago
Reviewers:
Weiliang, chunyang.dai
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

[x87] Keep x87 FPU stack empty when calling or return to natvie C++ function. X87 TurboFan code generation convention assumes that there is always a value at the top of the X87 FPU stack for each TurboFan's float operation. But native C++ function assumes there are 8 FPU stack slots can be used when it's called. This will lead to FPU stack overflow when TurboFan x87 code calls or returns back to native C++ function. as there are only 7 FPU stack slots remained for this native C++ function. This CL does: 1. Make sure X87 FPU stack depth always 1 before each TurboFan's float operation 2. Remove the top value in X87 FPU stack required by TurboFan when calling or returning from TurboFan Functions to other TurboFan or Non-TurboFan Functions. 3. Add the strict X87 FPU stack depth check for TurboFan debug code. 4. Re-initialize the X87 FPU stack and push a value at the top of the X87 FPU stack to satify the X87 TurboFan code generation convention for float operation at the entries where the TurboFan code will be called such as: exception handler, CallCFunctions in tests,..etc BUG= Committed: https://crrev.com/17bf607ddc6b7e6d040d0dbd775fa316f56864cc Cr-Commit-Position: refs/heads/master@{#33573}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+135 lines, -0 lines) Patch
M src/compiler/x87/code-generator-x87.cc View 35 chunks +135 lines, -0 lines 0 comments Download

Messages

Total messages: 12 (6 generated)
zhengxing.li
PTAL, thanks!
4 years, 11 months ago (2016-01-27 09:21:13 UTC) #2
Weiliang
lgtm
4 years, 10 months ago (2016-01-28 07:07:43 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1636353002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1636353002/1
4 years, 10 months ago (2016-01-28 07:09:28 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1636353002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1636353002/1
4 years, 10 months ago (2016-01-28 07:23:09 UTC) #8
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 10 months ago (2016-01-28 07:32:48 UTC) #10
commit-bot: I haz the power
4 years, 10 months ago (2016-01-28 07:33:04 UTC) #12
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/17bf607ddc6b7e6d040d0dbd775fa316f56864cc
Cr-Commit-Position: refs/heads/master@{#33573}

Powered by Google App Engine
This is Rietveld 408576698