This is an archive of the discontinued LLVM Phabricator instance.

Add mrrc/mrrc2 intrinsics and update existing mcrr/mcrr2 intrinsics to accept a single uint64 type instead of 2 uint32 types
ClosedPublic

Authored by rs on Jun 9 2016, 5:07 AM.

Details

Summary

Patch adds support for the intrinsics mrrc/mrrc2 and updates the existing mcrr/mcrr2 intrinsic definition to accept a single uint64 type as the input value instead of 2 uint32's as I think this makes the definition more compact.

Diff Detail

Repository
rL LLVM

Event Timeline

rs updated this revision to Diff 60161.Jun 9 2016, 5:07 AM
rs retitled this revision from to Add mrrc/mrrc2 intrinsics and update existing mcrr/mcrr2 intrinsics to accept a single uint64 type instead of 2 uint32 types.
rs updated this object.
rs added reviewers: t.p.northover, rengolin.
rs added a subscriber: cfe-commits.

LLVM part of the patch is here http://reviews.llvm.org/D21178

rengolin added inline comments.Jun 10 2016, 10:09 AM
lib/CodeGen/CGBuiltin.cpp
3817 ↗(On Diff #60161)

Would be better to use the comments as names and avoid the comments...

It'd also make clear the casts below.

3824 ↗(On Diff #60161)

These variables could have better names...

3846 ↗(On Diff #60161)

Same here

test/CodeGen/builtins-arm.c
203 ↗(On Diff #60161)

I'm assuming this is a "soon to be documented" move, right?

It's really hard to review patches with these things changing all the time...

rs marked 3 inline comments as done.Jun 13 2016, 4:34 AM
rs added inline comments.
test/CodeGen/builtins-arm.c
203 ↗(On Diff #60161)

I'm assuming this is a "soon to be documented" move, right?

Yes I've written the document myself which defines these intrinsics.

It's really hard to review patches with these things changing all the time...

Sorry, I didn't know these sorts of thing change that often.

rs updated this revision to Diff 60504.Jun 13 2016, 4:36 AM

Hi Renato,

please look again at this patch, I've addressed your comments.

Thanks,
Ranjeet

rengolin accepted this revision.Jun 15 2016, 3:54 AM
rengolin edited edge metadata.

Thanks Ranjeet, looks much better now! LGTM, Thanks!

This revision is now accepted and ready to land.Jun 15 2016, 3:54 AM
This revision was automatically updated to reflect the committed changes.