[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

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

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

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

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

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

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 meta-instruction and return the result.
+/// This assumes that \p It is not at the end of a block.
+BasicBlock::iterator skipMetaInstructions(BasicBlock::iterator It);
+
 } // end namespace llvm
 
 #endif // LLVM_IR_BASICBLOCK_H

include/llvm/IR/Instruction.h

bool isEHPad() const {
     case Instruction::CleanupPad:
     case Instruction::LandingPad:
       return true;
     default:
       return false;
     }
   }
 
+  /// Return true if the instruction has no effect other than describing
+  /// program properties.
+  ///
+  /// Meta-instructions do not affect program execution and are ignored by the
+  /// optimizer.
+  ///
+  /// This includes llvm.dbg.*, llvm.donothing, and llvm.*.annotation.*.
+  bool isMeta() const;
+
+  /// Return a pointer to the next non-meta instruction in the same basic
+  /// block as 'this', or nullptr if no such instruction exists.
+  const Instruction *getNextNonMetaInstruction() const;
+  Instruction *getNextNonMetaInstruction() {
+    return const_cast<Instruction *>(
+        static_cast<const Instruction *>(this)->getNextNonMetaInstruction());
+  }
+
   /// 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

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::skipMetaInstructions(BasicBlock::iterator It) {
+  while (It->isMeta())
+    ++It;
+  return It;
+}

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);
 }
 
+bool Instruction::isMeta() const {
+  auto *II = dyn_cast<IntrinsicInst>(this);
+  if (!II)
+    return false;
+
+  if (isa<DbgInfoIntrinsic>(II))
+    return true;
+
+  switch (II->getIntrinsicID()) {
+  default:
+    return false;
+  case Intrinsic::donothing:
+  case Intrinsic::annotation:
+  case Intrinsic::var_annotation:
+  case Intrinsic::ptr_annotation:
+    return true;
+  }
+}
+
+const Instruction *Instruction::getNextNonMetaInstruction() const {
+  for (const Instruction *I = getNextNode(); I; I = I->getNextNode())
+    if (!I->isMeta())
+      return I;
+  return nullptr;
+}
+
 bool Instruction::isAssociative() const {
   unsigned Opcode = getOpcode();
   if (isAssociative(Opcode))
     return true;
 
   switch (Opcode) {
   case FMul:
   case FAdd:

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 meta-instructions.
+        if (SS->getNextNonMetaInstruction() == 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.getNextNonMetaInstruction();
   if (auto *NFI = dyn_cast<FenceInst>(Next))
     if (FI.isIdenticalTo(NFI))
       return eraseInstFromFunction(FI);
   return nullptr;
 }
 
 // InvokeInst simplification
 Instruction *InstCombiner::visitInvokeInst(InvokeInst &II) {

lib/Transforms/Scalar/LoopRerollPass.cpp

for (auto *U : I->users()) {
     for (auto It = Start; It != End; ++It)
       if (U == It->first)
         return true;
   }
   return false;
 }
 
 static bool isIgnorableInst(const Instruction *I) {
-  if (isa<DbgInfoIntrinsic>(I))
-    return true;
-  const IntrinsicInst* II = dyn_cast<IntrinsicInst>(I);
-  if (!II)
-    return false;
-  switch (II->getIntrinsicID()) {
-    default:
-      return false;
-    case Intrinsic::annotation:
-    case Intrinsic::ptr_annotation:
-    case Intrinsic::var_annotation:
-    // TODO: the following intrinsics may also be whitelisted:
-    //   lifetime_start, lifetime_end, invariant_start, invariant_end
-      return true;
-  }
-  return false;
+  return I->isMeta();
 }
 
 bool LoopReroll::DAGRootTracker::validate(ReductionTracker &Reductions) {
   // We now need to check for equivalence of the use graph of each root with
   // that of the primary induction variable (excluding the roots). Our goal
   // here is not to solve the full graph isomorphism problem, but rather to
   // catch common cases without a lot of work. As a result, we will assume
   // that the relative order of the instructions in each unrolled iteration

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, CommuteShuffleMask) {
   SmallVector<int, 16> Indices({-1, 0, 7});
   ShuffleVectorInst::commuteShuffleMask(Indices, 4);
   EXPECT_THAT(Indices, testing::ContainerEq(ArrayRef<int>({-1, 4, 3})));
 }
 
+TEST(InstructionsTest, SkipMeta) {
+  LLVMContext C;
+  std::unique_ptr<Module> M = parseIR(C,
+                                      R"(
+      declare void @llvm.dbg.value(metadata, metadata, metadata)
+      declare void @llvm.donothing()
+      declare i32 @llvm.annotation.i32(i32, i8*, i8*, i32)
+      declare void @llvm.var.annotation(i8*, i8*, i8*, i32)
+      declare i8* @llvm.ptr.annotation.p0i8(i8*, i8*, i8*, i32)
+
+      define void @f() {
+      entry:
+        call void @llvm.dbg.value(metadata i32 0, metadata !11, metadata !DIExpression()), !dbg !13
+        call void @llvm.donothing()
+        %0 = call i32 @llvm.annotation.i32(i32 0, i8* undef, i8* undef, i32 0)
+        call void @llvm.var.annotation(i8* undef, i8* undef, i8* undef, i32 0)
+        %1 = call i8* @llvm.ptr.annotation.p0i8(i8* undef, i8* undef, i8* undef, i32 0)
+        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)
+      !14 = !DILocation(line: 3, column: 1, scope: !8)
+  )");
+  ASSERT_TRUE(M);
+  Function *F = cast<Function>(M->getNamedValue("f"));
+  BasicBlock &BB = F->front();
+
+  // The first non-meta instruction is the terminator.
+  auto *Term = BB.getTerminator();
+  EXPECT_EQ(Term, BB.begin()->getNextNonMetaInstruction());
+  EXPECT_EQ(Term->getIterator(), skipMetaInstructions(BB.begin()));
+
+  // After the terminator, there are no non-meta instructions.
+  EXPECT_EQ(nullptr, Term->getNextNonMetaInstruction());
+}
+
 } // end anonymous namespace
 } // end namespace llvm