[NFC][Metadata] Change MDNode::operands()'s return type from op_range to ArrayRef<MDOperand>

[yurai007]

Summary

This patch is https://reviews.llvm.org/D129468 follow-up and address one of comment coming from that review: https://reviews.llvm.org/D129468#3643295

[dexonsmith]

llvm/include/llvm/IR/Metadata.h

1348

I’m surprised SmallVector doesn’t have a generic constructor from ArrayRef. I wonder if it should?

(why did the iterator_range work before, but ArrayRef doesn’t?)

[yurai007]

llvm/include/llvm/IR/Metadata.h

1348

That's because SmallVector has appropriate constructor:

template <typename RangeTy>
explicit SmallVector(const iterator_range<RangeTy> &R);

but is lacking one for ArrayRef. However from what I can see by browsing containers definitions in llvm/ADT, convenience constructors taking ArrayRefs are not very popular. There is only TinyPtrVector which has one. But the same can be said for iterator_range - only SmallVector constructor takes it.
Even if constructing SmallVector from ArrayRef's iterators seem to not be extremely popular, still, I can find some places in code base (except ones from this change ofc. ) that would benefit from having convenience constructor - files where ArrayRef mask is used like X86ISelLowering.cpp, HexagonISelDAGToDAGHVX.cpp and DAGCombiner.cpp.

[dexonsmith]

This looks good to me, but it’d be nice to clean up further somehow later, maybe by adding a new free function.

llvm/include/llvm/IR/Metadata.h

1348

I was wondering if to_vector<Metadata *>(operands()) would work here, but it doesn’t have a template for the value type. We’d need a new thing, to_vector_of<>, which took the desired value type as the first parameter.

[yurai007]

In D129565#3648665, @dexonsmith wrote:

This looks good to me, but it’d be nice to clean up further somehow later, maybe by adding a new free function.

Thanks for the review! Sure, I'm gonna continue cleaning up this with to_vector_of in next patch.

[yurai007]

Add main one and missing operands() usage in TypeBasedAliasAnalysis

llvm/include/llvm/IR/Metadata.h

public:
   op_iterator op_begin() const {
     return const_cast<MDNode *>(this)->mutable_begin();
   }
 
   op_iterator op_end() const {
     return const_cast<MDNode *>(this)->mutable_end();
   }
 
-  op_range operands() const { return op_range(op_begin(), op_end()); }
+  ArrayRef<MDOperand> operands() const { return getHeader().operands(); }
 
   const MDOperand &getOperand(unsigned I) const {
     assert(I < getNumOperands() && "Out of range");
     return getHeader().operands()[I];
   }
 
   /// Return number of MDNode operands.
   unsigned getNumOperands() const { return getHeader().getNumOperands(); }
class MDTuple : public MDNode {
 
   void setHash(unsigned Hash) { SubclassData32 = Hash; }
   void recalculateHash();
 
   static MDTuple *getImpl(LLVMContext &Context, ArrayRef<Metadata *> MDs,
                           StorageType Storage, bool ShouldCreate = true);
 
   TempMDTuple cloneImpl() const {
-    return getTemporary(getContext(), SmallVector<Metadata *, 4>(operands()));
+    ArrayRef<MDOperand> Operands = operands();
+    return getTemporary(getContext(), SmallVector<Metadata *, 4>(
+                                          Operands.begin(), Operands.end()));
   }
 
 public:
   /// Get the hash, if any.
   unsigned getHash() const { return SubclassData32; }
 
   static MDTuple *get(LLVMContext &Context, ArrayRef<Metadata *> MDs) {
     return getImpl(Context, MDs, Uniqued);

llvm/lib/Analysis/TypeBasedAliasAnalysis.cpp

TBAAStructTypeNode getFieldType(unsigned FieldIndex) const {
     auto *TypeNode = cast<MDNode>(getNode()->getOperand(OpIndex));
     return TBAAStructTypeNode(TypeNode);
   }
 
   /// Get this TBAAStructTypeNode's field in the type DAG with
   /// given offset. Update the offset to be relative to the field type.
   TBAAStructTypeNode getField(uint64_t &Offset) const {
     bool NewFormat = isNewFormat();
-    const ArrayRef<MDOperand> Operands(Node->op_begin(), Node->op_end());
+    const ArrayRef<MDOperand> Operands = Node->operands();
     const unsigned NumOperands = Operands.size();
 
     if (NewFormat) {
       // New-format root and scalar type nodes have no fields.
       if (NumOperands < 6)
         return TBAAStructTypeNode();
     } else {
       // Parent can be omitted for the root node.
MDNode *AAMDNodes::extendToTBAA(MDNode *MD, ssize_t Len) {
   if (!Tag.isNewFormat())
     return MD;
 
   // If unknown size, drop the TBAA.
   if (Len == -1)
     return nullptr;
 
   // Otherwise, create TBAA with the new Len
-  SmallVector<Metadata *, 4> NextNodes(MD->operands());
+  ArrayRef<MDOperand> MDOperands = MD->operands();
+  SmallVector<Metadata *, 4> NextNodes(MDOperands.begin(), MDOperands.end());
   ConstantInt *PreviousSize = mdconst::extract<ConstantInt>(NextNodes[3]);
 
   // Don't create a new MDNode if it is the same length.
   if (PreviousSize->equalsInt(Len))
     return MD;
 
   NextNodes[3] =
       ConstantAsMetadata::get(ConstantInt::get(PreviousSize->getType(), Len));
   return MDNode::get(MD->getContext(), NextNodes);
 }

llvm/lib/Linker/IRMover.cpp

if (SrcBehaviorValue != DstBehaviorValue) {
                          DstM.getModuleIdentifier() + "'");
     }
 
     auto ensureDistinctOp = [&](MDNode *DstValue) {
       assert(isa<MDTuple>(DstValue) &&
              "Expected MDTuple when appending module flags");
       if (DstValue->isDistinct())
         return dyn_cast<MDTuple>(DstValue);
+      ArrayRef<MDOperand> DstOperands = DstValue->operands();
       MDTuple *New = MDTuple::getDistinct(
-          DstM.getContext(), SmallVector<Metadata *, 4>(DstValue->operands()));
+          DstM.getContext(),
+          SmallVector<Metadata *, 4>(DstOperands.begin(), DstOperands.end()));
       Metadata *FlagOps[] = {DstOp->getOperand(0), ID, New};
       MDNode *Flag = MDTuple::getDistinct(DstM.getContext(), FlagOps);
       DstModFlags->setOperand(DstIndex, Flag);
       Flags[ID].first = Flag;
       return New;
     };
 
     // Emit a warning if the values differ and either source or destination