[IR] Introduce helpers to skip debug instructions (NFC)

[vsk]

Summary

This patch introduces two helpers to make it easier to ignore debug
intrinsics:

• Instruction::getNextNonDebugInstruction()

This is just like Instruction::getNextNode(), except that it skips debug
info.

• skipDebugInfo(BasicBlock::iterator)

A free function which advances a BasicBlock iterator past any debug
info. This is a no-op when the iterator already points to a non-debug
instruction.

Part of: llvm.org/PR37728
Related to: https://reviews.llvm.org/D47874

[aprantl]

lib/IR/Instruction.cpp

603 ↗

(On Diff #151822)

I think it would be more efficient to include these cases in the switch below?

[efriedma]

I don't think donothing counts as a meta-instruction, by your definition; yes, it has no effects and no operands, but it isn't ignored by the optimizer (it generally just gets erased, but that's not the same as ignoring it).

llvm.annotation and llvm.ptr.annotation have return values, so they can't be ignored by optimizations, in general. llvm.var.annotation has a pointer operand, so it also can't be ignored by optimizations, in general. This is intentional, so they can be used to experiment with new optimizations.

Given that, a "meta-instruction" is precisely debug info. And I don't think that will change in the future.

[vsk]

In D48305#1136018, @efriedma wrote:

I don't think donothing counts as a meta-instruction, by your definition; yes, it has no effects and no operands, but it isn't ignored by the optimizer (it generally just gets erased, but that's not the same as ignoring it).

Ah, you're right. The langref explains that calls to llvm.donothing may be removed, but aren't generally ignored.

llvm.annotation and llvm.ptr.annotation have return values, so they can't be ignored by optimizations, in general. llvm.var.annotation has a pointer operand, so it also can't be ignored by optimizations, in general. This is intentional, so they can be used to experiment with new optimizations.

Given that, a "meta-instruction" is precisely debug info. And I don't think that will change in the future.

Thanks for providing some background on what llvm.*.annotation is meant to do. My previous understanding of these instructions was that they shouldn't affect whether or not an optimization triggers (the langref states: "they are ignored by code generation and optimization"). I think I was reading too much into this. The language isn't the same as what's used for debug intrinsics ("debug information does not prevent optimizations from happening"). Some optimizations do fail to "kick-in" when annotations are present (jump threading, a few combines), but I suppose that's expected.

Based on this feedback, I think it'd make sense to go with @fhahn's original approach (i.e just introduce a skipDebugInstructions helper).

[efriedma]

lib/IR/Instruction.cpp

602 ↗

(On Diff #151945)

Maybe this should assert rather than return nullptr? It's probably a logic error if you're calling getNextNonDebugInstruction() on a terminator.

[vsk]

lib/IR/Instruction.cpp

602 ↗

(On Diff #151945)

Hm, but what about loops which advance an iterator using getNextNode()? Here's one from slp-vectorizer (at least, I think 'ScheduleEnd' can be nullptr here): for (auto *I = ScheduleStart; I != ScheduleEnd; I = I->getNextNode()). I'd like getNextNonDebugInst to be a drop-in in these cases.

[efriedma]

LGTM

lib/IR/Instruction.cpp

602 ↗

(On Diff #151945)

I don't think ScheduleEnd can actually be null there?

But I guess in general, it could be a useful idiom, sure.

llvm/trunk/include/llvm/IR/BasicBlock.h

private:
   void setValueSubclassData(unsigned short D) {
     Value::setValueSubclassData(D);
   }
 };
 
 // Create wrappers for C Binding types (see CBindingWrapping.h).
 DEFINE_SIMPLE_CONVERSION_FUNCTIONS(BasicBlock, LLVMBasicBlockRef)
 
+/// Advance \p It while it points to a debug instruction and return the result.
+/// This assumes that \p It is not at the end of a block.
+BasicBlock::iterator skipDebugInfo(BasicBlock::iterator It);
+
 } // end namespace llvm
 
 #endif // LLVM_IR_BASICBLOCK_H

llvm/trunk/include/llvm/IR/Instruction.h

bool isEHPad() const {
     case Instruction::CleanupPad:
     case Instruction::LandingPad:
       return true;
     default:
       return false;
     }
   }
 
+  /// Return a pointer to the next non-debug instruction in the same basic
+  /// block as 'this', or nullptr if no such instruction exists.
+  const Instruction *getNextNonDebugInstruction() const;
+  Instruction *getNextNonDebugInstruction() {
+    return const_cast<Instruction *>(
+        static_cast<const Instruction *>(this)->getNextNonDebugInstruction());
+  }
+
   /// Create a copy of 'this' instruction that is identical in all ways except
   /// the following:
   ///   * The instruction has no parent
   ///   * The instruction has no name
   ///
   Instruction *clone() const;
 
   /// Return true if the specified instruction is exactly identical to the

llvm/trunk/lib/IR/BasicBlock.cpp

if (MDNode *MDIrrLoopHeader =
     MDString *MDName = cast<MDString>(MDIrrLoopHeader->getOperand(0));
     if (MDName->getString().equals("loop_header_weight")) {
       auto *CI = mdconst::extract<ConstantInt>(MDIrrLoopHeader->getOperand(1));
       return Optional<uint64_t>(CI->getValue().getZExtValue());
     }
   }
   return Optional<uint64_t>();
 }
+
+BasicBlock::iterator llvm::skipDebugInfo(BasicBlock::iterator It) {
+  while (isa<DbgInfoIntrinsic>(It))
+    ++It;
+  return It;
+}

llvm/trunk/lib/IR/Instruction.cpp

 //===-- Instruction.cpp - Implement the Instruction class -----------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the Instruction class for the IR library.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/IR/Instruction.h"
+#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/Operator.h"
 #include "llvm/IR/Type.h"
 using namespace llvm;
 
bool Instruction::mayThrow() const {
   return isa<ResumeInst>(this);
 }
 
 bool Instruction::isSafeToRemove() const {
   return (!isa<CallInst>(this) || !this->mayHaveSideEffects()) &&
          !isa<TerminatorInst>(this);
 }
 
+const Instruction *Instruction::getNextNonDebugInstruction() const {
+  for (const Instruction *I = getNextNode(); I; I = I->getNextNode())
+    if (!isa<DbgInfoIntrinsic>(I))
+      return I;
+  return nullptr;
+}
+
 bool Instruction::isAssociative() const {
   unsigned Opcode = getOpcode();
   if (isAssociative(Opcode))
     return true;
 
   switch (Opcode) {
   case FMul:
   case FAdd:

llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp

case Intrinsic::amdgcn_update_dpp: {
     II->setOperand(0, UndefValue::get(Old->getType()));
     return II;
   }
   case Intrinsic::stackrestore: {
     // If the save is right next to the restore, remove the restore.  This can
     // happen when variable allocas are DCE'd.
     if (IntrinsicInst *SS = dyn_cast<IntrinsicInst>(II->getArgOperand(0))) {
       if (SS->getIntrinsicID() == Intrinsic::stacksave) {
-        // Skip over debug info instructions.
-        // FIXME: This should be an utility in Instruction.h
-        auto It = SS->getIterator();
-        It++;
-        while (isa<DbgInfoIntrinsic>(*It))
-          It++;
-        if (&*It == II) {
+        // Skip over debug info.
+        if (SS->getNextNonDebugInstruction() == II) {
           return eraseInstFromFunction(CI);
         }
       }
     }
 
     // Scan down this block to see if there is another stack restore in the
     // same block without an intervening call/alloca.
     BasicBlock::iterator BI(II);

   }
   }
   return visitCallSite(II);
 }
 
 // Fence instruction simplification
 Instruction *InstCombiner::visitFenceInst(FenceInst &FI) {
   // Remove identical consecutive fences.
-  Instruction *Next = FI.getNextNode();
-  while (Next != nullptr && isa<DbgInfoIntrinsic>(Next))
-    Next = Next->getNextNode();
-
+  Instruction *Next = FI.getNextNonDebugInstruction();
   if (auto *NFI = dyn_cast<FenceInst>(Next))
     if (FI.isIdenticalTo(NFI))
       return eraseInstFromFunction(FI);
   return nullptr;
 }
 
 // InvokeInst simplification
 Instruction *InstCombiner::visitInvokeInst(InvokeInst &II) {

llvm/trunk/unittests/IR/InstructionsTest.cpp

 //===- llvm/unittest/IR/InstructionsTest.cpp - Instructions unit tests ----===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
+#include "llvm/AsmParser/Parser.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/NoFolder.h"
 #include "llvm/IR/Operator.h"
+#include "llvm/Support/SourceMgr.h"
 #include "gmock/gmock-matchers.h"
 #include "gtest/gtest.h"
 #include <memory>
 
 namespace llvm {
 namespace {
 
+static std::unique_ptr<Module> parseIR(LLVMContext &C, const char *IR) {
+  SMDiagnostic Err;
+  std::unique_ptr<Module> Mod = parseAssemblyString(IR, Err, C);
+  if (!Mod)
+    Err.print("InstructionsTests", errs());
+  return Mod;
+}
+
 TEST(InstructionsTest, ReturnInst) {
   LLVMContext C;
 
   // test for PR6589
   const ReturnInst* r0 = ReturnInst::Create(C);
   EXPECT_EQ(r0->getNumOperands(), 0U);
   EXPECT_EQ(r0->op_begin(), r0->op_end());
 
TEST(InstructionsTest, ShuffleMaskQueries) {
 
   EXPECT_TRUE(ShuffleVectorInst::isZeroEltSplatMask(ConstantVector::get({C4, CU, CU, C4})));
   EXPECT_TRUE(ShuffleVectorInst::isZeroEltSplatMask(ConstantVector::get({CU, C0, CU, C0})));
 
   EXPECT_TRUE(ShuffleVectorInst::isTransposeMask(ConstantVector::get({C1, C5, C3, C7})));
   EXPECT_TRUE(ShuffleVectorInst::isTransposeMask(ConstantVector::get({C1, C3})));
 }
 
+TEST(InstructionsTest, SkipDebug) {
+  LLVMContext C;
+  std::unique_ptr<Module> M = parseIR(C,
+                                      R"(
+      declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+      define void @f() {
+      entry:
+        call void @llvm.dbg.value(metadata i32 0, metadata !11, metadata !DIExpression()), !dbg !13
+        ret void
+      }
+
+      !llvm.dbg.cu = !{!0}
+      !llvm.module.flags = !{!3, !4}
+      !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 6.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+      !1 = !DIFile(filename: "t2.c", directory: "foo")
+      !2 = !{}
+      !3 = !{i32 2, !"Dwarf Version", i32 4}
+      !4 = !{i32 2, !"Debug Info Version", i32 3}
+      !8 = distinct !DISubprogram(name: "f", scope: !1, file: !1, line: 1, type: !9, isLocal: false, isDefinition: true, scopeLine: 1, isOptimized: false, unit: !0, retainedNodes: !2)
+      !9 = !DISubroutineType(types: !10)
+      !10 = !{null}
+      !11 = !DILocalVariable(name: "x", scope: !8, file: !1, line: 2, type: !12)
+      !12 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+      !13 = !DILocation(line: 2, column: 7, scope: !8)
+  )");
+  ASSERT_TRUE(M);
+  Function *F = cast<Function>(M->getNamedValue("f"));
+  BasicBlock &BB = F->front();
+
+  // The first non-debug instruction is the terminator.
+  auto *Term = BB.getTerminator();
+  EXPECT_EQ(Term, BB.begin()->getNextNonDebugInstruction());
+  EXPECT_EQ(Term->getIterator(), skipDebugInfo(BB.begin()));
+
+  // After the terminator, there are no non-debug instructions.
+  EXPECT_EQ(nullptr, Term->getNextNonDebugInstruction());
+}
+
 } // end anonymous namespace
 } // end namespace llvm