Issue 1883903002: [Atomics] Handle conversion to SMI in builtin, not code stub.

Issue 1883903002: [Atomics] Handle conversion to SMI in builtin, not code stub. (Closed)

Created:
4 years, 8 months ago by binji

Modified:
4 years, 8 months ago

Reviewers:
Benedikt Meurer

CC:
v8-reviews_googlegroups.com, v8-mips-ports_googlegroups.com, v8-x87-ports_googlegroups.com, v8-ppc-ports_googlegroups.com

Base URL:
https://chromium.googlesource.com/v8/v8.git@master

Target Ref:
refs/pending/heads/master

Project:
v8

Visibility:
Public.

More Reviews

Description

[Atomics] Handle conversion to SMI in builtin, not code stub. This removes a nice chunk of code from each code stub. BUG=v8:4614 LOG=n R=bmeurer@chromium.org

Patch Set 2 : properly handle signed vs. unsigned #

Total comments: 1

Created: 4 years, 8 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+43 lines, -210 lines)			Patch
M	src/arm/code-stubs-arm.cc	View		2 chunks	+2 lines, -56 lines	0 comments	Download
M	src/arm64/code-stubs-arm64.cc	View		2 chunks	+1 line, -29 lines	0 comments	Download
M	src/builtins.cc	View	1	4 chunks	+36 lines, -4 lines	1 comment	Download
M	src/ia32/code-stubs-ia32.cc	View		2 chunks	+2 lines, -64 lines	0 comments	Download
M	src/x64/code-stubs-x64.cc	View		2 chunks	+2 lines, -57 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Benedikt Meurer

Nice cleanup. Just needs some massaging on the interface descriptors. https://codereview.chromium.org/1883903002/diff/20001/src/builtins.cc File src/builtins.cc (right): https://codereview.chromium.org/1883903002/diff/20001/src/builtins.cc#newcode5304 ...

4 years, 8 months ago (2016-04-14 04:16:20 UTC) #2

binji

4 years, 8 months ago (2016-04-14 22:00:14 UTC) #3

On 2016/04/14 04:16:20, Benedikt Meurer wrote:
> Nice cleanup. Just needs some massaging on the interface descriptors.
> 
> https://codereview.chromium.org/1883903002/diff/20001/src/builtins.cc
> File src/builtins.cc (right):
> 
>
https://codereview.chromium.org/1883903002/diff/20001/src/builtins.cc#newcode...
> src/builtins.cc:5304: Node* untagged_result =
> a->CallStub(atomics_load.descriptor(), target,
> I think this is not safe, because the CallInterfaceDescriptor claims that the
> result of the stub call is AnyTagged and therefore we will put this value into
> the pointer map, and a potential GC triggered by either ChangeInt32ToTagged or
> ChangeUint32ToTagged will blow up because the result looks like a HeapObject*,
> but is really just some integer.
> 
> The fix should be to change the return type of the AtomicLoadDescriptor to
> UntaggedIntegral32 in interface-descriptors.cc.

Discarding in favor of https://codereview.chromium.org/1891033002/

Expand Messages | Collapse Messages