Page MenuHomePhabricator

[RISCV 1/10] Recognise riscv32 and riscv64 in triple parsing code
ClosedPublic

Authored by asb on Aug 16 2016, 8:17 AM.

Details

Summary

I'm sharing an initial set of patches that incrementally add an MC layer for RISC-V to LLVM. I've split everything up in to what are hopefully easily reviewable chunks (much like has been done with the incremental import of the AVR backend).

Diff Detail

Repository
rL LLVM

Event Timeline

asb updated this revision to Diff 68181.Aug 16 2016, 8:17 AM
asb retitled this revision from to [RISCV 1/10] Recognise riscv32 and riscv64 in triple parsing code.
asb updated this object.
asb added reviewers: theraven, jyknight.
asb added a subscriber: llvm-commits.
jyknight edited edge metadata.Aug 16 2016, 1:14 PM

Before this can be committed to llvm, permission to contribute from everyone who's committed code to the upstream branch will need to be collected.

git log --format="%an <%ae>" llvm/master..riscv-llvm/riscv-trunk|sort |uniq -c
shows, it looks like, 11 people whose permission needs to be asked. Some of those only contributed a 1-line trivial fix, so possibly it's okay if they don't reply, but might as well try asking everyone first. Certainly the more major contributors need to say OK.

I believe that if you cc those authors on the proposal email for adding the backend to LLVM, and ask them to explicitly reply to the list that they're okay with their work in https://github.com/riscv/riscv-llvm getting contributed to LLVM, under the LLVM license, that will be sufficient.

It'd also be a good idea to put the names/emails of the relevant additional original authors in each patch.

asb added a comment.Aug 16 2016, 1:44 PM

Hi James. This series of patches is not derivative from the riscv-llvm repository, it is a new codebase taking the approach of starting from the MC layer and building up.

Aha; it wasn't clear to me that this was original work, totally independent of that repository. Thanks for the clarification!

emaste added a subscriber: emaste.Aug 16 2016, 2:31 PM
emaste added inline comments.Aug 16 2016, 2:34 PM
unittests/ADT/TripleTest.cpp
257–261 ↗(On Diff #68181)

Worth adding a "riscv64-unknown-freebsd" as well?

asb updated this revision to Diff 68317.EditedAug 17 2016, 12:30 AM
asb edited edge metadata.

Add riscv64-unknown-freebsd test as suggested by @emaste

jyknight accepted this revision.Aug 17 2016, 11:36 AM
jyknight edited edge metadata.
This revision is now accepted and ready to land.Aug 17 2016, 11:36 AM
asb updated this revision to Diff 69345.EditedAug 26 2016, 5:14 AM
asb edited edge metadata.
asb added a subscriber: bogner.

Fix incorrect sort order issue in lib/Support/Triple.cpp reported by @bogner (the email doesn't seem to have triggered a Phabricator update...)

If comparing the diffs, ignore the lost lines in TripleTest.cpp. This is an upstream change visible because I rebased my patchset and because I am following the LLVM code review advice of generating diffs with -U99999

theraven accepted this revision.Aug 26 2016, 6:00 AM
theraven edited edge metadata.
t.p.northover accepted this revision.Aug 26 2016, 2:37 PM
t.p.northover added a reviewer: t.p.northover.
t.p.northover added a subscriber: t.p.northover.

I think this is pretty straightforward now. Anyone wanting to support other OSs can put their own tests in there.

emaste added inline comments.Aug 26 2016, 6:07 PM
lib/Support/Triple.cpp
1310 ↗(On Diff #69345)

break misaligned here (but could just be phabricator's rendering)

asb updated this revision to Diff 69481.Aug 27 2016, 1:25 AM
asb edited edge metadata.
asb marked an inline comment as done.

Fix whitespace damage (thanks @emaste!)

This revision was automatically updated to reflect the committed changes.