[PATCH] D86269: [RFC][Target] Add a new backend target called CSKY

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added a comment.

This is a massive code drop; I don't think it will be possible for any one person to review.



================
Comment at: llvm/lib/Target/CSKY/CSKY.h:28
+
+void initializeCSKYConstantIslandsPass(PassRegistry &);
+
----------------
Give the parameter a name? Looks like it's used.


================
Comment at: llvm/lib/Target/CSKY/CSKYAsmPrinter.cpp:85-86
+
+  switch (Opc) {
+  case CSKY::LRWJT32: {
+    MCInst TmpInst;
----------------
Do you plan to add more `case`s or can this just be a simple `if` statement for now?


================
Comment at: llvm/lib/Target/CSKY/CSKYConstantIslandPass.cpp:284
+  unsigned getOffsetOf(MachineInstr *MI) const;
+  unsigned getUserOffset(CPUser &) const;
+  void dumpBBs();
----------------
name param


================
Comment at: llvm/lib/Target/CSKY/CSKYConstantIslandPass.cpp:384
+  unsigned NoCPIters = 0, NoBRIters = 0;
+  (void)NoBRIters;
+  while (true) {
----------------
?


================
Comment at: llvm/lib/Target/CSKY/CSKYConstantIslandPass.cpp:545
+  // instructions in the inline assembly.
+  for (MachineFunction::iterator I = MF->begin(), E = MF->end(); I != E; ++I)
+    computeBlockSize(&*I);
----------------
would a range-for work better here? (similar to what's used on L552)


================
Comment at: llvm/lib/Target/CSKY/CSKYConstantIslandPass.cpp:616
+          // Remember that this is a user of a CP entry.
+          unsigned CPI = MI.getOperand(Op).getIndex();
+          MachineInstr *CPEMI = CPEMIs[CPI];
----------------
I see `MI.getOperand(Op)` subexpression repeated.  I think we have a range-for over operands that might help here?


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

https://reviews.llvm.org/D86269