This is an archive of the discontinued LLVM Phabricator instance.

[libcxxabi][mips] Correct float_data::mangled_size for all ABI's.
ClosedPublic

Authored by dsanders on Jul 24 2015, 3:56 AM.

Details

Diff Detail

Repository
rL LLVM

Event Timeline

dsanders updated this revision to Diff 30568.Jul 24 2015, 3:56 AM
dsanders retitled this revision from to [mips] Correct float_data::mangled_size for all ABI's..
dsanders updated this object.
dsanders added subscribers: cfe-commits, hans, nitesh.jain and 2 others.

@mclow.lists: I couldn't find a libcxxabi code owner in CODE_OWNERS.txt. Do you cover libcxxabi as well as libcxx?

dsanders retitled this revision from [mips] Correct float_data::mangled_size for all ABI's. to [libcxxabi][mips] Correct float_data::mangled_size for all ABI's..Jul 24 2015, 3:58 AM
hans added a comment.Jul 27 2015, 1:24 PM

Ping? This on my 3.7 hotlist.

libc++abi is outside my area of knowledge but seeing as no other reviewers have come forward and the patch is MIPS specific I'd like to LGTM this on the basis that I've tested it and found that it fixes the bug in the release.

Hans: After committing, the process still needs the code owner to approve the merge. We haven't heard anything from the person we think is the code-owner so I'm not sure how to proceed. Would you be willing to approve it without the code-owner or would you prefer to wait?

I'd like to LGTM this on the basis

Scratch that. I'm one of the authors (albeit as a minor revision on Jaydeep+Nitesh's patch) and I can't approve my own patches :-).

Hans: Are you willing to LGTM it?

I have no objections to this change; but I have no way to test it.

hans added a comment.Jul 30 2015, 7:09 AM

I'd like to LGTM this on the basis

Scratch that. I'm one of the authors (albeit as a minor revision on Jaydeep+Nitesh's patch) and I can't approve my own patches :-).

Hans: Are you willing to LGTM it?

I don't know enough here to be a good review. What's mangled_size used for?

I have no objections to this change; but I have no way to test it.

Is that an lgtm or call for some kind of test? :-)

In D11483#215138, @hans wrote:

I have no objections to this change; but I have no way to test it.

Is that an lgtm or call for some kind of test? :-)

It looks good to me, but I don't have access to a MIPS or MIPS_n64 system.
If people with access to those systems are OK with this, I am as well.

This revision was automatically updated to reflect the committed changes.

Thanks Marshall. Committed in r243645.