IR: Remove uselist for constantdata

This is a resurrected version of the patch attached to this RFC:

https://discourse.llvm.org/t/rfc-constantdata-should-not-have-use-lists/42606

In this adaptation, there are a few differences. In the original patch, the Use's
use list was replaced with an unsigned* to the reference count in the value. This
version leaves them as null and leaves the ref counting only in Value.

The current blocker is IROutliner has its own diy cloning, which doesn't use
ValueMapper and it does make direct use of constant use lists.

Remove use-lists from instances of ConstantData (which are shared
across modules and have no operands).

To continue supporting most of the use-list API, store a ref-count in
place of the use-list; this is for API like Value::use_empty and
Value::hasNUses.  Operations that actually need the use-list -- like
Value::use_begin -- will assert.

This change has three benefits:

 1. The compiler output cannot in any way depend on the use-list order
    of instances of ConstantData.

 2. There's no use-list traffic when adding and removing simple
    constants from operand lists (although there is ref-count traffic;
    YMMV).

 3. It's cheaper to serialize use-lists (since we're no longer
    serializing the use-list order of things like i32 0).

The downside is that you can't look at all the users of ConstantData,
but traversals of users of i32 0 are already ill-advised.

Possible follow-ups:
  - Track if an instance of a ConstantVector/ConstantArray/etc. is known
    to have all ConstantData arguments, and drop the use-lists to
    ref-counts in those cases.  Callers need to check Value::hasUseList
    before iterating through the use-list.
  - Remove even the ref-counts.  I'm not sure they have any benefit
    besides minimizing the scope of this commit, and maintaining the
    counts is not free.

Fixes #58629

Author: arsenm

llvm/include/llvm/IR/Use.h

@@ -23,6 +23,7 @@
 namespace llvm {
 
 template <typename> struct simplify_type;
+class ConstantData;
 class User;
 class Value;
 
@@ -42,10 +43,7 @@ class Use {
 
 private:
   /// Destructor - Only for zap()
-  ~Use() {
-    if (Val)
-      removeFromList();
-  }
+  ~Use();
 
   /// Constructor
   Use(User *Parent) : Parent(Parent) {}
@@ -87,19 +85,10 @@ class Use {
   Use **Prev = nullptr;
   User *Parent = nullptr;
 
-  void addToList(Use **List) {
-    Next = *List;
-    if (Next)
-      Next->Prev = &Next;
-    Prev = List;
-    *Prev = this;
-  }
-
-  void removeFromList() {
-    *Prev = Next;
-    if (Next)
-      Next->Prev = Prev;
-  }
+  inline void addToList(unsigned &Count);
+  inline void addToList(Use *&List);
+  inline void removeFromList(unsigned &Count);
+  inline void removeFromList(Use *&List);
 };
 
 /// Allow clients to treat uses just like values when using

llvm/include/llvm/IR/Value.h

@@ -116,7 +116,10 @@ class Value {
 
 private:
   Type *VTy;
-  Use *UseList;
+  union {
+    Use *List = nullptr;
+    unsigned Count;
+  } Uses;
 
   friend class ValueAsMetadata; // Allow access to IsUsedByMD.
   friend class ValueHandleBase; // Allow access to HasValueHandle.
@@ -341,21 +344,28 @@ class Value {
 #endif
   }
 
+  /// Check if this Value has a use-list.
+  bool hasUseList() const { return !isa<ConstantData>(this); }
+
   bool use_empty() const {
     assertModuleIsMaterialized();
-    return UseList == nullptr;
+    return hasUseList() ? Uses.List == nullptr : Uses.Count == 0;
   }
 
   bool materialized_use_empty() const {
-    return UseList == nullptr;
+    return hasUseList() ? Uses.List == nullptr : !Uses.Count;
   }
 
   using use_iterator = use_iterator_impl<Use>;
   using const_use_iterator = use_iterator_impl<const Use>;
 
-  use_iterator materialized_use_begin() { return use_iterator(UseList); }
+  use_iterator materialized_use_begin() {
+    assert(hasUseList());
+    return use_iterator(Uses.List);
+  }
   const_use_iterator materialized_use_begin() const {
-    return const_use_iterator(UseList);
+    assert(hasUseList());
+    return const_use_iterator(Uses.List);
   }
   use_iterator use_begin() {
     assertModuleIsMaterialized();
@@ -382,17 +392,18 @@ class Value {
     return materialized_uses();
   }
 
-  bool user_empty() const {
-    assertModuleIsMaterialized();
-    return UseList == nullptr;
-  }
+  bool user_empty() const { return use_empty(); }
 
   using user_iterator = user_iterator_impl<User>;
   using const_user_iterator = user_iterator_impl<const User>;
 
-  user_iterator materialized_user_begin() { return user_iterator(UseList); }
+  user_iterator materialized_user_begin() {
+    assert(hasUseList());
+    return user_iterator(Uses.List);
+  }
   const_user_iterator materialized_user_begin() const {
-    return const_user_iterator(UseList);
+    assert(hasUseList());
+    return const_user_iterator(Uses.List);
   }
   user_iterator user_begin() {
     assertModuleIsMaterialized();
@@ -431,7 +442,11 @@ class Value {
   ///
   /// This is specialized because it is a common request and does not require
   /// traversing the whole use list.
-  bool hasOneUse() const { return hasSingleElement(uses()); }
+  bool hasOneUse() const {
+    if (!hasUseList())
+      return Uses.Count == 1;
+    return hasSingleElement(uses());
+  }
 
   /// Return true if this Value has exactly N uses.
   bool hasNUses(unsigned N) const;
@@ -493,6 +508,8 @@ class Value {
   static void dropDroppableUse(Use &U);
 
   /// Check if this value is used in the specified basic block.
+  ///
+  /// Not supported for ConstantData.
   bool isUsedInBasicBlock(const BasicBlock *BB) const;
 
   /// This method computes the number of uses of this Value.
@@ -502,7 +519,19 @@ class Value {
   unsigned getNumUses() const;
 
   /// This method should only be used by the Use class.
-  void addUse(Use &U) { U.addToList(&UseList); }
+  void addUse(Use &U) {
+    if (hasUseList())
+      U.addToList(Uses.List);
+    else
+      U.addToList(Uses.Count);
+  }
+
+  void removeUse(Use &U) {
+    if (hasUseList())
+      U.removeFromList(Uses.List);
+    else
+      U.removeFromList(Uses.Count);
+  }
 
   /// Concrete subclass of this.
   ///
@@ -843,7 +872,8 @@ class Value {
   ///
   /// \return the first element in the list.
   ///
-  /// \note Completely ignores \a Use::Prev (doesn't read, doesn't update).
+  /// \note Completely ignores \a Use::PrevOrCount (doesn't read, doesn't
+  /// update).
   template <class Compare>
   static Use *mergeUseLists(Use *L, Use *R, Compare Cmp) {
     Use *Merged;
@@ -889,10 +919,53 @@ inline raw_ostream &operator<<(raw_ostream &OS, const Value &V) {
   return OS;
 }
 
+inline Use::~Use() {
+  if (Val)
+    Val->removeUse(*this);
+}
+
+void Use::addToList(unsigned &Count) {
+  assert(isa<ConstantData>(Val) && "Only ConstantData is ref-counted");
+  ++Count;
+
+  // We don't have a uselist - clear the remnant if we are replacing a
+  // non-constant value.
+  Prev = nullptr;
+  Next = nullptr;
+}
+
+void Use::addToList(Use *&List) {
+  assert(!isa<ConstantData>(Val) && "ConstantData has no use-list");
+
+  Next = List;
+  if (Next)
+    Next->Prev = &Next;
+  Prev = &List;
+  List = this;
+}
+
+void Use::removeFromList(unsigned &Count) {
+  assert(isa<ConstantData>(Val));
+  assert(Count > 0 && "reference count underflow");
+  assert(Prev == nullptr && "should not have uselist remnant");
+  assert(Next == nullptr && "should not have uselist remnant");
+  --Count;
+}
+
+void Use::removeFromList(Use *&List) {
+  *Prev = Next;
+  if (Next)
+    Next->Prev = Prev;
+}
+
 void Use::set(Value *V) {
-  if (Val) removeFromList();
+  if (Val) {
+    Val->removeUse(*this);
+  }
   Val = V;
-  if (V) V->addUse(*this);
+  if (V) {
+    V->addUse(*this);
+  }
 }
 
 Value *Use::operator=(Value *RHS) {
@@ -906,7 +979,7 @@ const Use &Use::operator=(const Use &RHS) {
 }
 
 template <class Compare> void Value::sortUseList(Compare Cmp) {
-  if (!UseList || !UseList->Next)
+  if (!hasUseList() || !Uses.List || !Uses.List->Next)
     // No need to sort 0 or 1 uses.
     return;
 
@@ -919,10 +992,10 @@ template <class Compare> void Value::sortUseList(Compare Cmp) {
   Use *Slots[MaxSlots];
 
   // Collect the first use, turning it into a single-item list.
-  Use *Next = UseList->Next;
-  UseList->Next = nullptr;
+  Use *Next = Uses.List->Next;
+  Uses.List->Next = nullptr;
   unsigned NumSlots = 1;
-  Slots[0] = UseList;
+  Slots[0] = Uses.List;
 
   // Collect all but the last use.
   while (Next->Next) {
@@ -958,15 +1031,15 @@ template <class Compare> void Value::sortUseList(Compare Cmp) {
   // Merge all the lists together.
   assert(Next && "Expected one more Use");
   assert(!Next->Next && "Expected only one Use");
-  UseList = Next;
+  Uses.List = Next;
   for (unsigned I = 0; I < NumSlots; ++I)
     if (Slots[I])
-      // Since the uses in Slots[I] originally preceded those in UseList, send
+      // Since the uses in Slots[I] originally preceded those in Uses.List, send
       // Slots[I] in as the left parameter to maintain a stable sort.
-      UseList = mergeUseLists(Slots[I], UseList, Cmp);
+      Uses.List = mergeUseLists(Slots[I], Uses.List, Cmp);
 
   // Fix the Prev pointers.
-  for (Use *I = UseList, **Prev = &UseList; I; I = I->Next) {
+  for (Use *I = Uses.List, **Prev = &Uses.List; I; I = I->Next) {
     I->Prev = Prev;
     Prev = &I->Next;
   }

llvm/lib/Analysis/TypeMetadataUtils.cpp

@@ -54,6 +54,9 @@ findCallsAtConstantOffset(SmallVectorImpl<DevirtCallSite> &DevirtCalls,
 static void findLoadCallsAtConstantOffset(
     const Module *M, SmallVectorImpl<DevirtCallSite> &DevirtCalls, Value *VPtr,
     int64_t Offset, const CallInst *CI, DominatorTree &DT) {
+  if (!VPtr->hasUseList())
+    return;
+
   for (const Use &U : VPtr->uses()) {
     Value *User = U.getUser();
     if (isa<BitCastInst>(User)) {

llvm/lib/AsmParser/LLParser.cpp

@@ -8863,6 +8863,8 @@ bool LLParser::parseMDNodeVector(SmallVectorImpl<Metadata *> &Elts) {
 //===----------------------------------------------------------------------===//
 bool LLParser::sortUseListOrder(Value *V, ArrayRef<unsigned> Indexes,
                                 SMLoc Loc) {
+  if (isa<ConstantData>(V))
+    return false;
   if (V->use_empty())
     return error(Loc, "value has no uses");
 

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

@@ -3857,6 +3857,10 @@ Error BitcodeReader::parseUseLists() {
         V = FunctionBBs[ID];
       } else
         V = ValueList[ID];
+
+      if (isa<ConstantData>(V))
+        break;
+
       unsigned NumUses = 0;
       SmallDenseMap<const Use *, unsigned, 16> Order;
       for (const Use &U : V->materialized_uses()) {

llvm/lib/Bitcode/Writer/ValueEnumerator.cpp

@@ -230,6 +230,9 @@ static void predictValueUseListOrderImpl(const Value *V, const Function *F,
 
 static void predictValueUseListOrder(const Value *V, const Function *F,
                                      OrderMap &OM, UseListOrderStack &Stack) {
+  if (isa<ConstantData>(V))
+    return;
+
   auto &IDPair = OM[V];
   assert(IDPair.first && "Unmapped value");
   if (IDPair.second)

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

@@ -3951,7 +3951,7 @@ static void emitGlobalConstantImpl(const DataLayout &DL, const Constant *CV,
   // Globals with sub-elements such as combinations of arrays and structs
   // are handled recursively by emitGlobalConstantImpl. Keep track of the
   // constant symbol base and the current position with BaseCV and Offset.
-  if (!BaseCV && CV->hasOneUse())
+  if (!isa<ConstantData>(CV) && !BaseCV && CV->hasOneUse())
     BaseCV = dyn_cast<Constant>(CV->user_back());
 
   if (isa<ConstantAggregateZero>(CV)) {

llvm/lib/CodeGen/CodeGenPrepare.cpp

@@ -8547,6 +8547,9 @@ static bool optimizeBranch(BranchInst *Branch, const TargetLowering &TLI,
     return false;
 
   Value *X = Cmp->getOperand(0);
+  if (!X->hasUseList())
+    return false;
+
   APInt CmpC = cast<ConstantInt>(Cmp->getOperand(1))->getValue();
 
   for (auto *U : X->users()) {

llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp

@@ -1034,6 +1034,9 @@ ComplexDeinterleavingGraph::identifyPartialReduction(Value *R, Value *I) {
   if (!isa<VectorType>(R->getType()) || !isa<VectorType>(I->getType()))
     return nullptr;
 
+  if (!R->hasUseList() || !I->hasUseList())
+    return nullptr;
+
   auto CommonUser =
       findCommonBetweenCollections<Value *>(R->users(), I->users());
   if (!CommonUser)

llvm/lib/IR/AsmWriter.cpp

@@ -125,11 +125,15 @@ static void orderValue(const Value *V, OrderMap &OM) {
   if (OM.lookup(V))
     return;
 
-  if (const Constant *C = dyn_cast<Constant>(V))
+  if (const Constant *C = dyn_cast<Constant>(V)) {
+    if (isa<ConstantData>(C))
+      return;
+
     if (C->getNumOperands() && !isa<GlobalValue>(C))
       for (const Value *Op : C->operands())
         if (!isa<BasicBlock>(Op) && !isa<GlobalValue>(Op))
           orderValue(Op, OM);
+  }
 
   // Note: we cannot cache this lookup above, since inserting into the map
   // changes the map's size, and thus affects the other IDs.
@@ -251,7 +255,8 @@ static UseListOrderMap predictUseListOrder(const Module *M) {
   UseListOrderMap ULOM;
   for (const auto &Pair : OM) {
     const Value *V = Pair.first;
-    if (V->use_empty() || std::next(V->use_begin()) == V->use_end())
+    if (!V->hasUseList() || V->use_empty() ||
+        std::next(V->use_begin()) == V->use_end())
       continue;
 
     std::vector<unsigned> Shuffle =

llvm/lib/IR/Instruction.cpp

@@ -373,7 +373,9 @@ std::optional<BasicBlock::iterator> Instruction::getInsertionPointAfterDef() {
 }
 
 bool Instruction::isOnlyUserOfAnyOperand() {
-  return any_of(operands(), [](Value *V) { return V->hasOneUser(); });
+  return any_of(operands(), [](const Value *V) {
+    return V->hasUseList() && V->hasOneUser();
+  });
 }
 
 void Instruction::setHasNoUnsignedWrap(bool b) {

llvm/lib/IR/Use.cpp

@@ -19,11 +19,15 @@ void Use::swap(Use &RHS) {
   std::swap(Next, RHS.Next);
   std::swap(Prev, RHS.Prev);
 
-  *Prev = this;
+  if (Prev)
+    *Prev = this;
+
   if (Next)
     Next->Prev = &Next;
 
-  *RHS.Prev = &RHS;
+  if (RHS.Prev)
+    *RHS.Prev = &RHS;
+
   if (RHS.Next)
     RHS.Next->Prev = &RHS.Next;
 }