[PATCH] D112206: [CSKY] First patch to construct codegen infra and generate first add instruction

[Renato Golin via Phabricator via llvm-commits]

rengolin added a comment.

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



================
Comment at: llvm/lib/Target/CSKY/CSKYFrameLowering.cpp:49
+
+void CSKYFrameLowering::emitPrologue(MachineFunction &MF,
+                                     MachineBasicBlock &MBB) const {}
----------------
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. :)


================
Comment at: llvm/lib/Target/CSKY/CSKYISelDAGToDAG.cpp:60
+
+  switch (Opcode) {
+  default:
----------------
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.


================
Comment at: llvm/lib/Target/CSKY/MCTargetDesc/CSKYBaseInfo.h:26
+// instruction info tracks. All definitions must match CSKYInstrFormats.td.
+namespace CSKYII {
+
----------------
What's the relation of this with `CSKYInstrInfo`?

How do other targets share that kind of stuff?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112206/new/

https://reviews.llvm.org/D112206