[DebugInfo][NFC] Add RawLocationWrapper to wrap location operand metadata [1/x]

[Orlando]

Summary

RawLocationWrapper wraps the location operand of a debug intrinsic which may be either (wrapped in MetadataAsValue) a DIArgList, ValueAsMetadata, or an empty MDTuple. This class lets us avoid duplicating location handling code in a later patch in this stack.

[StephenTozer]

Stack LGTM, seems like a quite straightforward set of changes.

[Orlando]

Seems like a quite straightforward set of changes.

I was surprised how few changes were required to get this working, which is always a nice feeling.

Thanks for the reviews.

llvm/include/llvm/IR/IntrinsicInst.h

static bool classof(const IntrinsicInst *I) {
     return isDbgInfoIntrinsic(I->getIntrinsicID());
   }
   static bool classof(const Value *V) {
     return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
   }
   /// @}
 };
 
-/// This is the common base class for debug info intrinsics for variables.
-class DbgVariableIntrinsic : public DbgInfoIntrinsic {
-public:
-  // Iterator for ValueAsMetadata that internally uses direct pointer iteration
-  // over either a ValueAsMetadata* or a ValueAsMetadata**, dereferencing to the
-  // ValueAsMetadata .
-  class location_op_iterator
-      : public iterator_facade_base<location_op_iterator,
-                                    std::bidirectional_iterator_tag, Value *> {
-    PointerUnion<ValueAsMetadata *, ValueAsMetadata **> I;
 
-  public:
-    location_op_iterator(ValueAsMetadata *SingleIter) : I(SingleIter) {}
-    location_op_iterator(ValueAsMetadata **MultiIter) : I(MultiIter) {}
 
-    location_op_iterator(const location_op_iterator &R) : I(R.I) {}
-    location_op_iterator &operator=(const location_op_iterator &R) {
-      I = R.I;
-      return *this;
-    }
-    bool operator==(const location_op_iterator &RHS) const {
-      return I == RHS.I;
-    }
-    const Value *operator*() const {
-      ValueAsMetadata *VAM = I.is<ValueAsMetadata *>()
-                                 ? I.get<ValueAsMetadata *>()
-                                 : *I.get<ValueAsMetadata **>();
-      return VAM->getValue();
-    };
-    Value *operator*() {
-      ValueAsMetadata *VAM = I.is<ValueAsMetadata *>()
-                                 ? I.get<ValueAsMetadata *>()
-                                 : *I.get<ValueAsMetadata **>();
-      return VAM->getValue();
-    }
-    location_op_iterator &operator++() {
-      if (I.is<ValueAsMetadata *>())
-        I = I.get<ValueAsMetadata *>() + 1;
-      else
-        I = I.get<ValueAsMetadata **>() + 1;
-      return *this;
-    }
-    location_op_iterator &operator--() {
-      if (I.is<ValueAsMetadata *>())
-        I = I.get<ValueAsMetadata *>() - 1;
-      else
-        I = I.get<ValueAsMetadata **>() - 1;
-      return *this;
-    }
-  };
+  bool operator==(const location_op_iterator &RHS) const { return I == RHS.I; }
 
+/// Lightweight class that wraps the location operand metadata of a debug
+/// intrinsic. The raw location may be a ValueAsMetadata, an empty MDTuple,
+/// or a DIArgList.
+class RawLocationWrapper {
+  Metadata *RawLocation = nullptr;
+
+public:
+  RawLocationWrapper() = default;
+  explicit RawLocationWrapper(Metadata *RawLocation)
+      : RawLocation(RawLocation) {
+    // Allow ValueAsMetadata, empty MDTuple, DIArgList.
+    assert(RawLocation && "unexpected null RawLocation");
+    assert(isa<ValueAsMetadata>(RawLocation) || isa<DIArgList>(RawLocation) ||
+           (isa<MDNode>(RawLocation) &&
+            !cast<MDNode>(RawLocation)->getNumOperands()));
+  }
+  Metadata *getRawLocation() const { return RawLocation; }
+  /// Get the locations corresponding to the variable referenced by the debug
+  /// info intrinsic.  Depending on the intrinsic, this could be the
+  /// variable's value or its address.
+  iterator_range<location_op_iterator> location_ops() const;
+  Value *getVariableLocationOp(unsigned OpIdx) const;
+  unsigned getNumVariableLocationOps() const {
+    if (hasArgList())
+      return cast<DIArgList>(getRawLocation())->getArgs().size();
+    return 1;
+  }
+  bool hasArgList() const { return isa<DIArgList>(getRawLocation()); }
+  bool isKillLocation(const DIExpression *Expression) const {
+    return (getNumVariableLocationOps() == 0 && !Expression->isComplex()) ||
+           any_of(location_ops(), [](Value *V) { return isa<UndefValue>(V); });
+  }
+
+  friend bool operator==(const RawLocationWrapper &A,
+                         const RawLocationWrapper &B) {
+    return A.RawLocation == B.RawLocation;
+  }
+  friend bool operator!=(const RawLocationWrapper &A,
+                         const RawLocationWrapper &B) {
+    return !(A == B);
+  }
+  friend bool operator>(const RawLocationWrapper &A,
+                        const RawLocationWrapper &B) {
+    return A.RawLocation > B.RawLocation;
+  }
+  friend bool operator>=(const RawLocationWrapper &A,
+                         const RawLocationWrapper &B) {
+    return A.RawLocation >= B.RawLocation;
+  }
+  friend bool operator<(const RawLocationWrapper &A,
+                        const RawLocationWrapper &B) {
+    return A.RawLocation < B.RawLocation;
+  }
+  friend bool operator<=(const RawLocationWrapper &A,
+                         const RawLocationWrapper &B) {
+    return A.RawLocation <= B.RawLocation;
+  }
+};
+
+/// This is the common base class for debug info intrinsics for variables.
+class DbgVariableIntrinsic : public DbgInfoIntrinsic {
+public:
   /// Get the locations corresponding to the variable referenced by the debug
   /// info intrinsic.  Depending on the intrinsic, this could be the
   /// variable's value or its address.
   iterator_range<location_op_iterator> location_ops() const;
 
   Value *getVariableLocationOp(unsigned OpIdx) const;
 
   void replaceVariableLocationOp(Value *OldValue, Value *NewValue);
   void replaceVariableLocationOp(unsigned OpIdx, Value *NewValue);
   /// Adding a new location operand will always result in this intrinsic using
   /// an ArgList, and must always be accompanied by a new expression that uses
   /// the new operand.
   void addVariableLocationOps(ArrayRef<Value *> NewValues,
                               DIExpression *NewExpr);
 
   void setVariable(DILocalVariable *NewVar) {
     setArgOperand(1, MetadataAsValue::get(NewVar->getContext(), NewVar));
   }
 
   void setExpression(DIExpression *NewExpr) {
     setArgOperand(2, MetadataAsValue::get(NewExpr->getContext(), NewExpr));
   }
 
   unsigned getNumVariableLocationOps() const {
-    if (hasArgList())
-      return cast<DIArgList>(getRawLocation())->getArgs().size();
-    return 1;
+    return getWrappedLocation().getNumVariableLocationOps();
   }
 
-  bool hasArgList() const { return isa<DIArgList>(getRawLocation()); }
+  bool hasArgList() const { return getWrappedLocation().hasArgList(); }
 
   /// Does this describe the address of a local variable. True for dbg.declare,
   /// but not dbg.value, which describes its value, or dbg.assign, which
   /// describes a combination of the variable's value and address.
   bool isAddressOfVariable() const {
     return getIntrinsicID() == Intrinsic::dbg_declare;
   }
 
   void setKillLocation() {
     // TODO: When/if we remove duplicate values from DIArgLists, we don't need
     // this set anymore.
     SmallPtrSet<Value *, 4> RemovedValues;
     for (Value *OldValue : location_ops()) {
       if (!RemovedValues.insert(OldValue).second)
         continue;
       Value *Poison = PoisonValue::get(OldValue->getType());
       replaceVariableLocationOp(OldValue, Poison);
     }
   }
 
   bool isKillLocation() const {
-    return (getNumVariableLocationOps() == 0 &&
-            !getExpression()->isComplex()) ||
-           any_of(location_ops(), [](Value *V) { return isa<UndefValue>(V); });
+    return getWrappedLocation().isKillLocation(getExpression());
   }
 
   DILocalVariable *getVariable() const {
     return cast<DILocalVariable>(getRawVariable());
   }
 
   DIExpression *getExpression() const {
     return cast<DIExpression>(getRawExpression());
   }
 
   Metadata *getRawLocation() const {
     return cast<MetadataAsValue>(getArgOperand(0))->getMetadata();
   }
 
+  RawLocationWrapper getWrappedLocation() const {
+    return RawLocationWrapper(getRawLocation());
+  }
+
   Metadata *getRawVariable() const {
     return cast<MetadataAsValue>(getArgOperand(1))->getMetadata();
   }
 
   Metadata *getRawExpression() const {
     return cast<MetadataAsValue>(getArgOperand(2))->getMetadata();
   }
 

llvm/lib/IR/IntrinsicInst.cpp

bool IntrinsicInst::mayLowerToFunctionCall(Intrinsic::ID IID) {
   }
 }
 
 //===----------------------------------------------------------------------===//
 /// DbgVariableIntrinsic - This is the common base class for debug info
 /// intrinsics for variables.
 ///
 
-iterator_range<DbgVariableIntrinsic::location_op_iterator>
-DbgVariableIntrinsic::location_ops() const {
-  auto *MD = getRawLocation();
+iterator_range<location_op_iterator> RawLocationWrapper::location_ops() const {
+  Metadata *MD = getRawLocation();
   assert(MD && "First operand of DbgVariableIntrinsic should be non-null.");
-
   // If operand is ValueAsMetadata, return a range over just that operand.
   if (auto *VAM = dyn_cast<ValueAsMetadata>(MD)) {
     return {location_op_iterator(VAM), location_op_iterator(VAM + 1)};
   }
   // If operand is DIArgList, return a range over its args.
   if (auto *AL = dyn_cast<DIArgList>(MD))
     return {location_op_iterator(AL->args_begin()),
             location_op_iterator(AL->args_end())};
   // Operand must be an empty metadata tuple, so return empty iterator.
   return {location_op_iterator(static_cast<ValueAsMetadata *>(nullptr)),
           location_op_iterator(static_cast<ValueAsMetadata *>(nullptr))};
 }
 
+iterator_range<location_op_iterator>
+DbgVariableIntrinsic::location_ops() const {
+  return getWrappedLocation().location_ops();
+}
+
 Value *DbgVariableIntrinsic::getVariableLocationOp(unsigned OpIdx) const {
-  auto *MD = getRawLocation();
+  return getWrappedLocation().getVariableLocationOp(OpIdx);
+}
+
+Value *RawLocationWrapper::getVariableLocationOp(unsigned OpIdx) const {
+  Metadata *MD = getRawLocation();
   assert(MD && "First operand of DbgVariableIntrinsic should be non-null.");
   if (auto *AL = dyn_cast<DIArgList>(MD))
     return AL->getArgs()[OpIdx]->getValue();
   if (isa<MDNode>(MD))
     return nullptr;
   assert(
       isa<ValueAsMetadata>(MD) &&
       "Attempted to get location operand from DbgVariableIntrinsic with none.");