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

Issue 1745643002: PPC: [turbofan] Don't use the CompareIC in JSGenericLowering. (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: [turbofan] Don't use the CompareIC in JSGenericLowering. Port d00da47b61462681b48e48bdff4a80a33da1a6d6 Original commit message: The CompareICStub produces an untagged raw word value, which has to be translated to true or false manually in the TurboFan code. But for lazy bailout after the CompareIC, we immediately go back to fullcodegen or Ignition with the raw value, to a location where both fullcodegen and Ignition expect a boolean value, which might crash or in the worst case (depending on the exact computation inside the CompareIC) could lead to arbitrary memory access. Short-term fix is to use the proper runtime functions (unified with the interpreter now) for comparisons. Next task is to provide optimized versions of these based on the CodeStubAssembler, which can then be used via code stubs in TurboFan or directly in handlers in the interpreter. R=bmeurer@chromium.org, joransiu@ca.ibm.com, jyan@ca.ibm.com, michael_dawson@ca.ibm.com BUG=v8:4788 LOG=n Committed: https://crrev.com/c1507e158780b170f25f0f1da87cb5d78a56faee Cr-Commit-Position: refs/heads/master@{#34341}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -1 line) Patch
M src/ppc/code-stubs-ppc.cc View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 7 (2 generated)
MTBrandyberry
4 years, 10 months ago (2016-02-26 21:54:12 UTC) #1
michael_dawson
On 2016/02/26 21:54:12, MTBrandyberry wrote: lgtm
4 years, 10 months ago (2016-02-26 21:55:11 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1745643002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1745643002/1
4 years, 10 months ago (2016-02-26 21:55:42 UTC) #4
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 10 months ago (2016-02-26 22:13:53 UTC) #5
commit-bot: I haz the power
4 years, 10 months ago (2016-02-26 22:15:03 UTC) #7
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/c1507e158780b170f25f0f1da87cb5d78a56faee
Cr-Commit-Position: refs/heads/master@{#34341}

Powered by Google App Engine
This is Rietveld 408576698