[gicombiner] Import tryCombineIndexedLoadStore()

[dsanders]

Summary

Now that arbitrary data is supported, import tryCombineIndexedLoadStore()

Depends on D69147

[volkan]

LGTM.

[dsanders]

rebase

[zatrazz]

Hi,

I am investigating a recurring regression on arm-linux bots and bisecting the commits on the build [1] that introduced the regression it points to this one. By reverting I can build the stage1 (I am testing now the stage2). I seems to only happen on arm-linux bots, I didn't see it on aarch64-linux ones.

[1] http://lab.llvm.org:8011/builders/clang-cmake-armv7-selfhost/builds/3251

llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h

 class MachineDominatorTree;
 
 struct PreferredTuple {
   LLT Ty;                // The result type of the extend.
   unsigned ExtendOpcode; // G_ANYEXT/G_SEXT/G_ZEXT
   MachineInstr *MI;
 };
 
+struct IndexedLoadStoreMatchInfo {
+  Register Addr;
+  Register Base;
+  Register Offset;
+  bool IsPre;
+};
+
 class CombinerHelper {
 protected:
   MachineIRBuilder &Builder;
   MachineRegisterInfo &MRI;
   GISelChangeObserver &Observer;
   GISelKnownBits *KB;
   MachineDominatorTree *MDT;
 
public:
   /// Returns true if MI changed.
   bool tryCombineExtendingLoads(MachineInstr &MI);
   bool matchCombineExtendingLoads(MachineInstr &MI, PreferredTuple &MatchInfo);
   void applyCombineExtendingLoads(MachineInstr &MI, PreferredTuple &MatchInfo);
 
   /// Combine \p MI into a pre-indexed or post-indexed load/store operation if
   /// legal and the surrounding code makes it useful.
   bool tryCombineIndexedLoadStore(MachineInstr &MI);
+  bool matchCombineIndexedLoadStore(MachineInstr &MI, IndexedLoadStoreMatchInfo &MatchInfo);
+  void applyCombineIndexedLoadStore(MachineInstr &MI, IndexedLoadStoreMatchInfo &MatchInfo);
 
   bool matchElideBrByInvertingCond(MachineInstr &MI);
   void applyElideBrByInvertingCond(MachineInstr &MI);
   bool tryElideBrByInvertingCond(MachineInstr &MI);
 
   /// If \p MI is G_CONCAT_VECTORS, try to combine it.
   /// Returns true if MI changed.
   /// Right now, we support:

llvm/include/llvm/Target/GlobalISel/Combine.td

 
 /// Declares data that is passed from the match stage to the apply stage.
 class GIDefMatchData<string type> : GIDefKind {
   /// A C++ type name indicating the storage type.
   string Type = type;
 }
 
 def extending_load_matchdata : GIDefMatchData<"PreferredTuple">;
+def indexed_load_store_matchdata : GIDefMatchData<"IndexedLoadStoreMatchInfo">;
 
 /// The operator at the root of a GICombineRule.Match dag.
 def match;
 /// All arguments of the match operator must be either:
 /// * A subclass of GIMatchKind
 /// * A subclass of GIMatchKindWithArgs
 /// * A subclass of Instruction
 /// * A MIR code block (deprecated)
def copy_prop : GICombineRule<
   (match [{ return Helper.matchCombineCopy(${d}); }]),
   (apply [{ Helper.applyCombineCopy(${d}); }])>;
 def trivial_combines : GICombineGroup<[copy_prop]>;
 
 def extending_loads : GICombineRule<
   (defs root:$root, extending_load_matchdata:$matchinfo),
   (match [{ return Helper.matchCombineExtendingLoads(${root}, ${matchinfo}); }]),
   (apply [{ Helper.applyCombineExtendingLoads(${root}, ${matchinfo}); }])>;
+def combines_for_extload: GICombineGroup<[extending_loads]>;
+
+def combine_indexed_load_store : GICombineRule<
+  (defs root:$root, indexed_load_store_matchdata:$matchinfo),
+  (match [{ return Helper.matchCombineIndexedLoadStore(${root}, ${matchinfo}); }]),
+  (apply [{ Helper.applyCombineIndexedLoadStore(${root}, ${matchinfo}); }])>;
 
 // FIXME: Is there a reason this wasn't in tryCombine? I've left it out of
 //        all_combines because it wasn't there.
 def elide_br_by_inverting_cond : GICombineRule<
   (defs root:$d),
   (match [{ return Helper.matchElideBrByInvertingCond(${d}); }]),
   (apply [{ Helper.applyElideBrByInvertingCond(${d}); }])>;
 
-def combines_for_extload: GICombineGroup<[extending_loads]>;
-
-def all_combines : GICombineGroup<[trivial_combines, combines_for_extload]>;
+def all_combines : GICombineGroup<[trivial_combines, combines_for_extload, combine_indexed_load_store]>;

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

if (!dominates(MI, UseMI)) {
       return false;
     }
   }
 
   return true;
 }
 
 bool CombinerHelper::tryCombineIndexedLoadStore(MachineInstr &MI) {
+  IndexedLoadStoreMatchInfo MatchInfo;
+  if (matchCombineIndexedLoadStore(MI, MatchInfo)) {
+    applyCombineIndexedLoadStore(MI, MatchInfo);
+    return true;
+  }
+  return false;
+}
+
+bool CombinerHelper::matchCombineIndexedLoadStore(MachineInstr &MI, IndexedLoadStoreMatchInfo &MatchInfo) {
   unsigned Opcode = MI.getOpcode();
   if (Opcode != TargetOpcode::G_LOAD && Opcode != TargetOpcode::G_SEXTLOAD &&
       Opcode != TargetOpcode::G_ZEXTLOAD && Opcode != TargetOpcode::G_STORE)
     return false;
 
-  bool IsStore = Opcode == TargetOpcode::G_STORE;
-  Register Addr, Base, Offset;
-  bool IsPre = findPreIndexCandidate(MI, Addr, Base, Offset);
-  if (!IsPre && !findPostIndexCandidate(MI, Addr, Base, Offset))
+  MatchInfo.IsPre = findPreIndexCandidate(MI, MatchInfo.Addr, MatchInfo.Base,
+                                          MatchInfo.Offset);
+  if (!MatchInfo.IsPre &&
+      !findPostIndexCandidate(MI, MatchInfo.Addr, MatchInfo.Base,
+                              MatchInfo.Offset))
     return false;
 
+  return true;
+}
 
+void CombinerHelper::applyCombineIndexedLoadStore(
+    MachineInstr &MI, IndexedLoadStoreMatchInfo &MatchInfo) {
+  MachineInstr &AddrDef = *MRI.getUniqueVRegDef(MatchInfo.Addr);
+  MachineIRBuilder MIRBuilder(MI);
+  unsigned Opcode = MI.getOpcode();
+  bool IsStore = Opcode == TargetOpcode::G_STORE;
   unsigned NewOpcode;
   switch (Opcode) {
   case TargetOpcode::G_LOAD:
     NewOpcode = TargetOpcode::G_INDEXED_LOAD;
     break;
   case TargetOpcode::G_SEXTLOAD:
     NewOpcode = TargetOpcode::G_INDEXED_SEXTLOAD;
     break;
   case TargetOpcode::G_ZEXTLOAD:
     NewOpcode = TargetOpcode::G_INDEXED_ZEXTLOAD;
     break;
   case TargetOpcode::G_STORE:
     NewOpcode = TargetOpcode::G_INDEXED_STORE;
     break;
   default:
     llvm_unreachable("Unknown load/store opcode");
   }
 
-  MachineInstr &AddrDef = *MRI.getUniqueVRegDef(Addr);
-  MachineIRBuilder MIRBuilder(MI);
   auto MIB = MIRBuilder.buildInstr(NewOpcode);
   if (IsStore) {
-    MIB.addDef(Addr);
+    MIB.addDef(MatchInfo.Addr);
     MIB.addUse(MI.getOperand(0).getReg());
   } else {
     MIB.addDef(MI.getOperand(0).getReg());
-    MIB.addDef(Addr);
+    MIB.addDef(MatchInfo.Addr);
   }
 
-  MIB.addUse(Base);
-  MIB.addUse(Offset);
-  MIB.addImm(IsPre);
+  MIB.addUse(MatchInfo.Base);
+  MIB.addUse(MatchInfo.Offset);
+  MIB.addImm(MatchInfo.IsPre);
   MI.eraseFromParent();
   AddrDef.eraseFromParent();
 
   LLVM_DEBUG(dbgs() << "    Combinined to indexed operation");
-  return true;
 }
 
 bool CombinerHelper::matchElideBrByInvertingCond(MachineInstr &MI) {
   if (MI.getOpcode() != TargetOpcode::G_BR)
     return false;
 
   // Try to match the following:
   // bb1:

llvm/lib/Target/AArch64/AArch64PreLegalizerCombiner.cpp

case Intrinsic::memset: {
       return (!EnableMinSize) ? Helper.tryCombineMemCpyFamily(MI, MaxLen)
                               : false;
     }
     default:
       break;
     }
   }
 
-  if (Generated.tryCombineAll(Observer, MI, B))
+  if (Generated.tryCombineAll(Observer, MI, B, Helper))
     return true;
 
   switch (MI.getOpcode()) {
   case TargetOpcode::G_CONCAT_VECTORS:
     return Helper.tryCombineConcatVectors(MI);
   case TargetOpcode::G_SHUFFLE_VECTOR:
     return Helper.tryCombineShuffleVector(MI);
-  case TargetOpcode::G_LOAD:
-  case TargetOpcode::G_SEXTLOAD:
-  case TargetOpcode::G_ZEXTLOAD:
-  case TargetOpcode::G_STORE:
-    return Helper.tryCombineIndexedLoadStore(MI);
   }
 
   return false;
 }
 
 #define AARCH64PRELEGALIZERCOMBINERHELPER_GENCOMBINERHELPER_CPP
 #include "AArch64GenGICombiner.inc"
 #undef AARCH64PRELEGALIZERCOMBINERHELPER_GENCOMBINERHELPER_CPP

llvm/utils/TableGen/GICombinerEmitter.cpp

OS << "#ifdef " << Name.upper() << "_GENCOMBINERHELPER_H\n"
      << "public:\n"
      << "  bool parseCommandLineOption();\n"
      << "  bool isRuleDisabled(unsigned ID) const;\n"
      << "  bool setRuleDisabled(StringRef RuleIdentifier);\n"
      << "\n"
      << "  bool tryCombineAll(\n"
      << "    GISelChangeObserver &Observer,\n"
      << "    MachineInstr &MI,\n"
-     << "    MachineIRBuilder &B) const;\n"
+     << "    MachineIRBuilder &B,\n"
+     << "    CombinerHelper &Helper) const;\n"
      << "};\n\n";
 
   emitNameMatcher(OS);
 
   OS << "bool " << getClassName()
      << "::setRuleDisabled(StringRef RuleIdentifier) {\n"
      << "  std::pair<StringRef, StringRef> RangePair = "
         "RuleIdentifier.split('-');\n"
OS << "#ifdef " << Name.upper() << "_GENCOMBINERHELPER_CPP\n"
      << "    if (!setRuleDisabled(Identifier))\n"
      << "      return false;\n"
      << "  return true;\n"
      << "}\n\n";
 
   OS << "bool " << getClassName() << "::tryCombineAll(\n"
      << "    GISelChangeObserver &Observer,\n"
      << "    MachineInstr &MI,\n"
-     << "    MachineIRBuilder &B) const {\n"
-     << "  CombinerHelper Helper(Observer, B);\n"
+     << "    MachineIRBuilder &B,\n"
+     << "    CombinerHelper &Helper) const {\n"
      << "  MachineBasicBlock *MBB = MI.getParent();\n"
      << "  MachineFunction *MF = MBB->getParent();\n"
      << "  MachineRegisterInfo &MRI = MF->getRegInfo();\n"
      << "  (void)MBB; (void)MF; (void)MRI;\n\n";
 
   OS << "  // Match data\n";
   for (const auto &Rule : Rules)
     for (const auto &I : Rule->matchdata_decls())