[GIsel][CombinerHelper] Don't consider debug insts in dominance queries [3/10]

[vsk]

Summary

This fixes several issues where the presence of debug instructions could
disable certain combines, due to dominance queries finding uses/defs that
don't actually exist. Loosely depends on D78137.

[vsk]

retitle

[dsanders]

LGTM

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

public:
   /// If \p MI is COPY, try to combine it.
   /// Returns true if MI changed.
   bool tryCombineCopy(MachineInstr &MI);
   bool matchCombineCopy(MachineInstr &MI);
   void applyCombineCopy(MachineInstr &MI);
 
   /// Returns true if \p DefMI precedes \p UseMI or they are the same
   /// instruction. Both must be in the same basic block.
-  bool isPredecessor(MachineInstr &DefMI, MachineInstr &UseMI);
+  bool isPredecessor(const MachineInstr &DefMI, const MachineInstr &UseMI);
 
   /// Returns true if \p DefMI dominates \p UseMI. By definition an
   /// instruction dominates itself.
   ///
   /// If we haven't been provided with a MachineDominatorTree during
   /// construction, this function returns a conservative result that tracks just
   /// a single basic block.
-  bool dominates(MachineInstr &DefMI, MachineInstr &UseMI);
+  bool dominates(const MachineInstr &DefMI, const MachineInstr &UseMI);
 
   /// If \p MI is extend that consumes the result of a load, try to combine it.
   /// 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

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

bool CombinerHelper::matchCombineExtendingLoads(MachineInstr &MI,
   // relative type sizes. At the same time, pick an extend to use based on the
   // extend involved in the chosen type.
   unsigned PreferredOpcode = MI.getOpcode() == TargetOpcode::G_LOAD
                                  ? TargetOpcode::G_ANYEXT
                                  : MI.getOpcode() == TargetOpcode::G_SEXTLOAD
                                        ? TargetOpcode::G_SEXT
                                        : TargetOpcode::G_ZEXT;
   Preferred = {LLT(), PreferredOpcode, nullptr};
-  for (auto &UseMI : MRI.use_instructions(LoadValue.getReg())) {
+  for (auto &UseMI : MRI.use_nodbg_instructions(LoadValue.getReg())) {
     if (UseMI.getOpcode() == TargetOpcode::G_SEXT ||
         UseMI.getOpcode() == TargetOpcode::G_ZEXT ||
         UseMI.getOpcode() == TargetOpcode::G_ANYEXT) {
       Preferred = ChoosePreferredUse(Preferred,
                                      MRI.getType(UseMI.getOperand(0).getReg()),
                                      UseMI.getOpcode(), &UseMI);
     }
   }
for (auto *UseMO : Uses) {
     // This is free on many targets.
     InsertInsnsWithoutSideEffectsBeforeUse(Builder, MI, *UseMO, InsertTruncAt);
   }
 
   MI.getOperand(0).setReg(ChosenDstReg);
   Observer.changedInstr(MI);
 }
 
-bool CombinerHelper::isPredecessor(MachineInstr &DefMI, MachineInstr &UseMI) {
+bool CombinerHelper::isPredecessor(const MachineInstr &DefMI,
+                                   const MachineInstr &UseMI) {
+  assert(!DefMI.isDebugInstr() && !UseMI.isDebugInstr() &&
+         "shouldn't consider debug uses");
   assert(DefMI.getParent() == UseMI.getParent());
   if (&DefMI == &UseMI)
     return false;
 
   // Loop through the basic block until we find one of the instructions.
   MachineBasicBlock::const_iterator I = DefMI.getParent()->begin();
   for (; &*I != &DefMI && &*I != &UseMI; ++I)
     return &*I == &DefMI;
 
   llvm_unreachable("Block must contain instructions");
 }
 
-bool CombinerHelper::dominates(MachineInstr &DefMI, MachineInstr &UseMI) {
+bool CombinerHelper::dominates(const MachineInstr &DefMI,
+                               const MachineInstr &UseMI) {
+  assert(!DefMI.isDebugInstr() && !UseMI.isDebugInstr() &&
+         "shouldn't consider debug uses");
   if (MDT)
     return MDT->dominates(&DefMI, &UseMI);
   else if (DefMI.getParent() != UseMI.getParent())
     return false;
 
   return isPredecessor(DefMI, UseMI);
 }
 
#endif
 
   Base = MI.getOperand(1).getReg();
   MachineInstr *BaseDef = MRI.getUniqueVRegDef(Base);
   if (BaseDef && BaseDef->getOpcode() == TargetOpcode::G_FRAME_INDEX)
     return false;
 
   LLVM_DEBUG(dbgs() << "Searching for post-indexing opportunity for: " << MI);
 
-  for (auto &Use : MRI.use_instructions(Base)) {
+  for (auto &Use : MRI.use_nodbg_instructions(Base)) {
     if (Use.getOpcode() != TargetOpcode::G_PTR_ADD)
       continue;
 
     Offset = Use.getOperand(2).getReg();
     if (!ForceLegalIndexing &&
         !TLI.isIndexingLegal(MI, Base, Offset, /*IsPre*/ false, MRI)) {
       LLVM_DEBUG(dbgs() << "    Ignoring candidate with illegal addrmode: "
                         << Use);
if (!OffsetDef || !dominates(*OffsetDef, MI)) {
       continue;
     }
 
     // FIXME: check whether all uses of Base are load/store with foldable
     // addressing modes. If so, using the normal addr-modes is better than
     // forming an indexed one.
 
     bool MemOpDominatesAddrUses = true;
-    for (auto &PtrAddUse : MRI.use_instructions(Use.getOperand(0).getReg())) {
+    for (auto &PtrAddUse :
+         MRI.use_nodbg_instructions(Use.getOperand(0).getReg())) {
       if (!dominates(MI, PtrAddUse)) {
         MemOpDominatesAddrUses = false;
         break;
       }
     }
 
     if (!MemOpDominatesAddrUses) {
       LLVM_DEBUG(
if (MI.getOperand(0).getReg() == Addr) {
       LLVM_DEBUG(dbgs() << "    Skipping, does not dominate all addr uses");
       return false;
     }
   }
 
   // FIXME: check whether all uses of the base pointer are constant PtrAdds.
   // That might allow us to end base's liveness here by adjusting the constant.
 
-  for (auto &UseMI : MRI.use_instructions(Addr)) {
+  for (auto &UseMI : MRI.use_nodbg_instructions(Addr)) {
     if (!dominates(MI, UseMI)) {
       LLVM_DEBUG(dbgs() << "    Skipping, does not dominate all addr uses.");
       return false;
     }
   }
 
   return true;
 }

llvm/test/CodeGen/AArch64/GlobalISel/combiner-load-store-indexing.ll

-; RUN: llc -mtriple=arm64-apple-ios -global-isel -global-isel-abort=1 -verify-machineinstrs -stop-after=aarch64-prelegalizer-combiner -force-legal-indexing %s -o - | FileCheck %s
+; RUN: llc -debugify-and-strip-all-safe -mtriple=arm64-apple-ios -global-isel -global-isel-abort=1 -verify-machineinstrs -stop-after=aarch64-prelegalizer-combiner -force-legal-indexing %s -o - | FileCheck %s
 
 define i8* @test_simple_load_pre(i8* %ptr) {
 ; CHECK-LABEL: name: test_simple_load_pre
 ; CHECK: [[BASE:%.*]]:_(p0) = COPY $x0
 ; CHECK: [[OFFSET:%.*]]:_(s64) = G_CONSTANT i64 42
 ; CHECK-NOT: G_PTR_ADD
 ; CHECK: {{%.*}}:_(s8), [[NEXT:%.*]]:_(p0) = G_INDEXED_LOAD [[BASE]], [[OFFSET]](s64), 1
 ; CHECK: $x0 = COPY [[NEXT]](p0)