This is an archive of the discontinued LLVM Phabricator instance.

[CSKY] First patch to construct codegen infra and generate first add instruction
ClosedPublic

Authored by zixuan-wu on Oct 21 2021, 2:31 AM.

Details

Summary

Ooops. It constructs codegen infra and provide only basic code to generate first add instruction successfully.

Diff Detail

Event Timeline

zixuan-wu created this revision.Oct 21 2021, 2:31 AM
zixuan-wu requested review of this revision.Oct 21 2021, 2:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 21 2021, 2:31 AM

Some general comments but this looks good as a first lowering of simple functions.

llvm/lib/Target/CSKY/CSKYFrameLowering.cpp
50

I know this will be filled soon, but doesn't hurt to put a FIXME comment saying "implement this when we have function calls" or something, so that if someone picks up the tree with this patch and not the next (for whatever reason), they don't think that CSKY has no prologue/epilogue. :)

llvm/lib/Target/CSKY/CSKYISelDAGToDAG.cpp
61

Same here, it's always good to add a small comment as to what may show in places that are obviously empty. This looks unnecessary when you look at your own tree, but given that your patches will have hundreds (if not thousands) of patches in between in the upstream tree, it's good practice to add more comments of the unfinished parts.

llvm/lib/Target/CSKY/MCTargetDesc/CSKYBaseInfo.h
27

What's the relation of this with CSKYInstrInfo?

How do other targets share that kind of stuff?

zixuan-wu updated this revision to Diff 381821.Oct 24 2021, 7:44 PM

Address comments.

zixuan-wu marked 2 inline comments as done.Oct 24 2021, 7:46 PM
zixuan-wu added inline comments.
llvm/lib/Target/CSKY/MCTargetDesc/CSKYBaseInfo.h
27

Sorry to can't understand what you mean. I think this comments are similar meaning as RISCVBaseInfo.h has.

rengolin accepted this revision.Oct 26 2021, 3:00 AM

Thanks again, this looks good to me now. Please wait a day or so to merge, in case there are any more reviews.

llvm/lib/Target/CSKY/MCTargetDesc/CSKYBaseInfo.h
27

I see, I was comparing this to the Arm/x86 back-ends. Last time I worked on this level was too long ago, and there is precedent, so this should be fine. :)

This revision is now accepted and ready to land.Oct 26 2021, 3:00 AM
zixuan-wu added inline comments.Oct 26 2021, 4:06 AM
llvm/lib/Target/CSKY/MCTargetDesc/CSKYBaseInfo.h
27

It's some flags to distinguish instruction type such as addressing mode of instruction.

This revision was landed with ongoing or failed builds.Oct 31 2021, 7:08 PM
This revision was automatically updated to reflect the committed changes.