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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/include/llvm/ADT/Triple.h:59
     bpfeb,          // eBPF or extended BPF or 64-bit BPF (big endian)
+    csky,           // CSKY: csky
     hexagon,        // Hexagon: hexagon
----------------
Usually the triple patch is split into a separate patch


================
Comment at: llvm/lib/Target/CSKY/CSKYInstrInfo.cpp:82
+  } else {
+    llvm_unreachable("Unimplemented yet");
+  }
----------------
Grammatically weird error throughout


================
Comment at: llvm/lib/Target/CSKY/CSKYMachineFunctionInfo.h:35
+public:
+  CSKYMachineFunctionInfo(MachineFunction &MF) : MF(MF) {}
+
----------------
I'm trying to remove the dependency on machine function state in the constructor in D80249


================
Comment at: llvm/lib/Target/CSKY/CSKYRegisterInfo.cpp:73
+  MachineInstr *NewMI = nullptr;
+  switch (MI->getDesc().getOpcode()) {
+  default:
----------------
Can do just MI->getOpcode()


================
Comment at: llvm/lib/Target/CSKY/CSKYRegisterInfo.cpp:78
+
+    unsigned NewReg = MRI.createVirtualRegister(&CSKY::GPRRegClass);
+
----------------
Register


================
Comment at: llvm/lib/Target/CSKY/CSKYRegisterInfo.cpp:149
+
+  if (1) {
+    if (RegOp != nullptr)
----------------
Constant if


================
Comment at: llvm/lib/Target/CSKY/CSKYSubtarget.cpp:45-47
+  std::string CPUName = std::string(CPU);
+  if (CPU.empty())
+    CPUName = "generic-csky";
----------------
No reason to use std:: string here


================
Comment at: llvm/lib/Target/CSKY/CSKYTargetMachine.cpp:98-116
+  Attribute CPUAttr = F.getFnAttribute("target-cpu");
+  Attribute FSAttr = F.getFnAttribute("target-features");
+
+  std::string CPU = !CPUAttr.hasAttribute(Attribute::None)
+                        ? CPUAttr.getValueAsString().str()
+                        : TargetCPU;
+  std::string FS = !FSAttr.hasAttribute(Attribute::None)
----------------
We should move this logic into the base implementation with a template argument for the subtarget class; I think every target reproduces approximately the same thing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86269