[PATCH] D97783: Add the Connex SIMD/vector processor back end (main back end patch)

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

There's way too much commented out and macro conditional code



================
Comment at: llvm/include/llvm/ADT/Triple.h:59
     bpfeb,          // eBPF or extended BPF or 64-bit BPF (big endian)
+    connex,         // Connex vector processor
     csky,           // CSKY: csky
----------------
Belongs with the other patch?


================
Comment at: llvm/lib/Target/Connex/Connex.td:30-42
+
+
+
+
+
+
+
----------------
Lots of extra spaces?


================
Comment at: llvm/lib/Target/Connex/ConnexAsmPrinter.cpp:64-66
+const MachineInstr *crtMI;
+extern std::unordered_map<const MachineInstr *, const MachineInstr *>
+    mapLD_ST_REPEAT_InlineAsm;
----------------
Can't have globals like this


================
Comment at: llvm/lib/Target/Connex/ConnexAsmPrinter.cpp:610
+  void replaceWithSymbolicIndex(MachineBasicBlock *MBB) {
+    assert(0 && "replaceWithSymbolicIndex() does NOT do anything anymore");
+
----------------
dead function


================
Comment at: llvm/lib/Target/Connex/ConnexAsmPrinter.cpp:1068
+
+   #ifdef ADAPTED_RPO
+    return finishingTimeMBB[&b1] > finishingTimeMBB[&b2];
----------------
Remove all of these type of macros


================
Comment at: llvm/lib/Target/Connex/ConnexAsmPrinter.cpp:1411-1431
+/*
+// From [LLVM]/llvm38Nov2016/llvm/lib/Target/Mips/MipsAsmPrinter.cpp
+void ConnexAsmPrinter::printUnsignedImm(const MachineInstr *MI, int opNum,
+                                      raw_ostream &O) {
+  const MachineOperand &MO = MI->getOperand(opNum);
+  if (MO.isImm())
+    O << (unsigned short int)MO.getImm();
----------------
Drop dead code


================
Comment at: llvm/lib/Target/Connex/ConnexAsmPrinterLoopNests.h:70-78
+    if (fgets(str, MAXLEN_STR - 1, fin) == NULL)
+      break;
+
+    printf("str = %s\n", str);
+    fflush(stdout);
+
+    // We read the coordinates of the innermost loop of the crt nest
----------------
Definitely shouldn't be using fgets/fscanf anywhere. File usage anywhere like this is dubious


================
Comment at: llvm/lib/Target/Connex/ConnexISelDAGToDAG.cpp:483
+        // if (crtValue->getName().str() == NVEC_STR)
+        if (strncmp(crtValue->getName().str().c_str(), NVEC_STR,
+                    strlen(NVEC_STR)) == 0) {
----------------
no strncmps


================
Comment at: llvm/lib/Target/Connex/ConnexISelDAGToDAG.cpp:592-593
+  //  }
+  char *exprStrChar = (char *)malloc(MAXLEN_STR);
+  strcpy(exprStrChar, asmString.c_str());
+  LLVM_DEBUG(dbgs() << "CreateInlineAsmNode(): exprStrChar = "
----------------
no raw mallocs and strcpy 


================
Comment at: llvm/lib/Target/Connex/Select_REDf16_OpincaaCodeGen.h:9-13
+// Code auto-generated by method Kernel::genLLVMISelManualCode()
+//   from the OPINCAA lib, from kernel red.f16.
+// You should include this code in the Select() method of the [Target]SelectionDAGISel
+//   class of your back end (or MAYBE in the ISelLowering pass).
+// Number of instructions generated: 122.
----------------
Generated code should only come from tablegen and not be committed


================
Comment at: llvm/test/CodeGen/Connex/MatMulBT-512_i16_tiled_182_74_512.ll:4
+; From ~/PPCG/Tests/35_MatMul/SIZE_512/P7/test_tiled.ll
+
+; ModuleID = 'test_tiled.scalar.ll'
----------------
This test is huge and I don't see any checks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97783