[ARM][NFC] Refactor sequential access for DSP

[samparker]

Summary

With a view to support parallel operations that have their results stored to memory, refactor the consecutive access helper out so it could support stores instructions.

[SjoerdMeijer]

Looks OK to me.

lib/Target/ARM/ARMParallelDSP.cpp

61

Nit: does the comment now needs updating? I.e., that it can be any instruction touching memory?

[samparker]

lib/Target/ARM/ARMParallelDSP.cpp

61

I don't think so. We're still expecting the ParallelMACs to operate directly on loaded data and that there aren't any stores in the chain. I will change and update this structure later - and I will try to remember to update the comments then!

lib/Target/ARM/ARMParallelDSP.cpp

 
 namespace {
   struct ParallelMAC;
   struct Reduction;
 
   using ParallelMACList = SmallVector<ParallelMAC, 8>;
   using ReductionList   = SmallVector<Reduction, 8>;
   using ValueList       = SmallVector<Value*, 8>;
-  using LoadInstList    = SmallVector<LoadInst*, 8>;
+  using MemInstList     = SmallVector<Instruction*, 8>;
   using PMACPair        = std::pair<ParallelMAC*,ParallelMAC*>;
   using PMACPairList    = SmallVector<PMACPair, 8>;
   using Instructions    = SmallVector<Instruction*,16>;
   using MemLocList      = SmallVector<MemoryLocation, 4>;
 
   // 'ParallelMAC' and 'Reduction' are just some bookkeeping data structures.
   // 'Reduction' contains the phi-node and accumulator statement from where we
   // start pattern matching, and 'ParallelMAC' the multiplication
   // instructions that are candidates for parallel execution.
   struct ParallelMAC {
     Instruction *Mul;
     ValueList    VL;        // List of all (narrow) operands of this Mul
-    LoadInstList VecLd;     // List of all load instructions of this Mul
+    MemInstList  VecLd;     // List of all load instructions of this Mul
     MemLocList   MemLocs;   // All memory locations read by this Mul
 
     ParallelMAC(Instruction *I, ValueList &V) : Mul(I), VL(V) {};
   };
 
   struct Reduction {
     PHINode         *Phi;             // The Phi-node from where we start
                                       // pattern matching.
class ARMParallelDSP : public LoopPass {
     TargetLibraryInfo *TLI;
     DominatorTree     *DT;
     LoopInfo          *LI;
     Loop              *L;
     const DataLayout  *DL;
     Module            *M;
 
     bool InsertParallelMACs(Reduction &Reduction, PMACPairList &PMACPairs);
-    bool AreSequentialLoads(LoadInst *Ld0, LoadInst *Ld1, LoadInstList &VecLd);
+    bool AreSequentialLoads(LoadInst *Ld0, LoadInst *Ld1, MemInstList &VecLd);
     PMACPairList CreateParallelMACPairs(ParallelMACList &Candidates);
     Instruction *CreateSMLADCall(LoadInst *VecLd0, LoadInst *VecLd1,
                                  Instruction *Acc, Instruction *InsertAfter);
 
     /// Try to match and generate: SMLAD, SMLADX - Signed Multiply Accumulate
     /// Dual performs two signed 16x16-bit multiplications. It adds the
     /// products to a 32-bit accumulate operand. Optionally, the instruction can
     /// exchange the halfwords of the second operand before performing the
for (unsigned i = 0; i < Pairs; ++i) {
     if (!(match(V0, m_APInt(C0)) && match(V1, m_APInt(C1)) && C0 == C1))
       return false;
   }
 
   LLVM_DEBUG(dbgs() << "OK: found symmetrical operand lists.\n");
   return true;
 }
 
+template<typename MemInst>
+static bool AreSequentialAccesses(MemInst *MemOp0, MemInst *MemOp1,
+                                  MemInstList &VecMem, const DataLayout &DL,
+                                  ScalarEvolution &SE) {
+  if (!MemOp0->isSimple() || !MemOp1->isSimple()) {
+    LLVM_DEBUG(dbgs() << "No, not touching volatile access\n");
+    return false;
+  }
+  if (isConsecutiveAccess(MemOp0, MemOp1, DL, SE)) {
+    VecMem.push_back(MemOp0);
+    VecMem.push_back(MemOp1);
+    LLVM_DEBUG(dbgs() << "OK: accesses are consecutive.\n");
+    return true;
+  }
+  LLVM_DEBUG(dbgs() << "No, accesses aren't consecutive.\n");
+  return false;
+}
+
 bool ARMParallelDSP::AreSequentialLoads(LoadInst *Ld0, LoadInst *Ld1,
-                                        LoadInstList &VecLd) {
+                                        MemInstList &VecMem) {
   if (!Ld0 || !Ld1)
     return false;
 
   LLVM_DEBUG(dbgs() << "Are consecutive loads:\n";
     dbgs() << "Ld0:"; Ld0->dump();
     dbgs() << "Ld1:"; Ld1->dump();
   );
 
-  if (!Ld0->isSimple() || !Ld1->isSimple()) {
-    LLVM_DEBUG(dbgs() << "No, not touching volatile loads\n");
-    return false;
-  }
   if (!Ld0->hasOneUse() || !Ld1->hasOneUse()) {
     LLVM_DEBUG(dbgs() << "No, load has more than one use.\n");
     return false;
   }
-  if (isConsecutiveAccess(Ld0, Ld1, *DL, *SE)) {
-    VecLd.push_back(Ld0);
-    VecLd.push_back(Ld1);
-    LLVM_DEBUG(dbgs() << "OK: loads are consecutive.\n");
-    return true;
-  }
-  LLVM_DEBUG(dbgs() << "No, Ld0 and Ld1 aren't consecutive.\n");
-  return false;
+
+  return AreSequentialAccesses<LoadInst>(Ld0, Ld1, VecMem, *DL, *SE);
 }
 
 PMACPairList
 ARMParallelDSP::CreateParallelMACPairs(ParallelMACList &Candidates) {
   const unsigned Elems = Candidates.size();
   PMACPairList PMACPairs;
 
   if (Elems < 2)
bool ARMParallelDSP::InsertParallelMACs(Reduction &Reduction,
                                         PMACPairList &PMACPairs) {
   Instruction *Acc = Reduction.Phi;
   Instruction *InsertAfter = Reduction.AccIntAdd;
 
   for (auto &Pair : PMACPairs) {
     LLVM_DEBUG(dbgs() << "Found parallel MACs!!\n";
                dbgs() << "- "; Pair.first->Mul->dump();
                dbgs() << "- "; Pair.second->Mul->dump());
-    Acc = CreateSMLADCall(Pair.first->VecLd[0], Pair.second->VecLd[0], Acc,
-                          InsertAfter);
+    auto *VecLd0 = cast<LoadInst>(Pair.first->VecLd[0]);
+    auto *VecLd1 = cast<LoadInst>(Pair.second->VecLd[0]);
+    Acc = CreateSMLADCall(VecLd0, VecLd1, Acc, InsertAfter);
     InsertAfter = Acc;
   }
 
   if (Acc != Reduction.Phi) {
     LLVM_DEBUG(dbgs() << "Replace Accumulate: "; Acc->dump());
     Reduction.AccIntAdd->replaceAllUsesWith(Acc);
     return true;
   }