Page MenuHomePhabricator

[CSKY 1/n] Add basic stub or infra of csky backend
ClosedPublic

Authored by zixuan-wu on Mon, Sep 28, 9:52 PM.

Details

Summary

This patch introduce files that just enough for lib/Target/CSKY to compile.
Notably a basic CSKYTargetMachine and CSKYTargetInfo.

Diff Detail

Event Timeline

zixuan-wu created this revision.Mon, Sep 28, 9:52 PM
Herald added a project: Restricted Project. · View Herald TranscriptMon, Sep 28, 9:52 PM
zixuan-wu requested review of this revision.Mon, Sep 28, 9:52 PM
zixuan-wu added a reviewer: craig.topper.

Gentle ping...
BTW, @lattner would you know who is the decision maker that can approve a new target and review related commit?

The general process is here: https://llvm.org/docs/DeveloperPolicy.html#adding-a-new-target

I'm sorry that I'm preoccupied with the devmtg this week, but please ping me next week if you're not getting traction, thanks!

Mostly boilerplate that looks fine, a few minor nits.

llvm/lib/Target/CSKY/CSKYTargetMachine.cpp
32

Minor nit, but hopefully anyone reading this knows what e means from reading the LangRef. It would be more useful for this comment to give a summary of what is happening here specifically, for example 'CSky is always little endian with a 4-byte aligned stack'

36

If the endian and stack alignment are the same for every instance of this back end, you don't need to add them separately. The same is true for all of the things after the mangling flag.

38

Does CSky really support all of the manglings that this can return? Including Windows x86 mangling? If not, please add some asserts to validate that it's a sane value.

71

Why make this a separate function instead of just writing this on line 84:

(RM.hasValue() ? *RM : Reloc::Static),

The general process is here: https://llvm.org/docs/DeveloperPolicy.html#adding-a-new-target

I'm sorry that I'm preoccupied with the devmtg this week, but please ping me next week if you're not getting traction, thanks!

The general process is clear but there is not info about who is main reviewer can approve threading patches.

zixuan-wu updated this revision to Diff 297119.Thu, Oct 8, 8:57 PM

Address comments.

zixuan-wu marked 4 inline comments as done.Thu, Oct 8, 8:57 PM
theraven accepted this revision.Fri, Oct 9, 1:06 AM

I lost track of the mailing list discussions, but assuming that there is consensus to accept this back end I am happy with the structure of the initial code, the size of the contributed diffs, and the engagement from the maintainer.

This revision is now accepted and ready to land.Fri, Oct 9, 1:06 AM

I lost track of the mailing list discussions, but assuming that there is consensus to accept this back end I am happy with the structure of the initial code, the size of the contributed diffs, and the engagement from the maintainer.

Thank you!

zixuan-wu edited the summary of this revision. (Show Details)Fri, Oct 9, 7:44 PM
This revision was automatically updated to reflect the committed changes.