Ooops. It constructs codegen infra and provide only basic code to generate first add instruction successfully.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
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? |
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. |
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. :) |
llvm/lib/Target/CSKY/MCTargetDesc/CSKYBaseInfo.h | ||
---|---|---|
27 | It's some flags to distinguish instruction type such as addressing mode of instruction. |
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. :)