[PATCH] D36331: Add ARC backend

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/ARC/ARCBranchFinalize.cpp:153-154
+    for (auto &MI : MBB) {
+      unsigned x = MI.getDesc().getSize();
+      if (x > 8 || x == 0) {
+        DEBUG(dbgs() << "Unknown (or size 0) size for: "; MI.dump();
----------------
You should implement and use TII::getInstSizeInBytes for this instead


================
Comment at: llvm/lib/Target/ARC/ARCBranchFinalize.cpp:173
+  DEBUG(dbgs() << "Estimated function size for "
+               << MF.getFunction()->getName().str() << ": " << maxSize << "\n");
+
----------------
.str() is unnecessary


================
Comment at: llvm/lib/Target/ARC/ARCExpandPseudos.cpp:79-80
+  for (auto &MBB : MF) {
+    for (auto &MI : MBB) {
+      switch (MI.getOpcode()) {
+      case ARC::ST_FAR:
----------------
You can avoid the ToRemove vector by directly using the iterator and incrementing it before you do the insert


================
Comment at: llvm/lib/Target/ARC/ARCExpandPseudos.cpp:97
+  }
+  return ToRemove.size() > 0;
+}
----------------
.empty()


================
Comment at: llvm/lib/Target/ARC/ARCISelLowering.cpp:187
+
+  if (LD->getAlignment() == 1) {
+    DEBUG(dbgs() << "Expanding align 1 load.\n");
----------------
You don't seem to implement allowsMisalignedMemoryAccesses which would get you a lot of this for free


================
Comment at: llvm/lib/Target/ARC/ARCISelLowering.cpp:255-256
+                                                  SelectionDAG &DAG) const {
+  DEBUG(dbgs() << "Customg lowering SIGN_EXTEND_INREG: "; Op.dump();
+        dbgs() << "\n");
+  SDValue Op0 = Op.getOperand(0);
----------------
I think you should remove any of these kinds of debug dumps. You already get these from the default selectionDAG debug output, so this is extra noise.


================
Comment at: llvm/lib/Target/ARC/ARCISelLowering.cpp:583
+      // Load the argument to a virtual register
+      unsigned ObjSize = VA.getLocVT().getSizeInBits() / 8;
+      assert((ObjSize <= StackSlotSize) && "Unhandled argument");
----------------
getStoreSize


https://reviews.llvm.org/D36331