[llvm-branch-commits] [llvm] IR: Remove uselist for constantdata (PR #134692)

Matt Arsenault via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Sat Apr 12 02:25:25 PDT 2025


https://github.com/arsenm updated https://github.com/llvm/llvm-project/pull/134692

>From 9313a3ec2e3cd0d4b68f4e3c02fe9bf05bbc2337 Mon Sep 17 00:00:00 2001
From: Matt Arsenault <Matthew.Arsenault at amd.com>
Date: Thu, 3 Apr 2025 17:30:01 +0700
Subject: [PATCH] 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.

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
---
 llvm/include/llvm/IR/Use.h                    |  23 +---
 llvm/include/llvm/IR/Value.h                  | 119 ++++++++++++++----
 llvm/lib/Analysis/TypeMetadataUtils.cpp       |   3 +
 llvm/lib/AsmParser/LLParser.cpp               |   2 +
 llvm/lib/Bitcode/Reader/BitcodeReader.cpp     |   4 +
 llvm/lib/Bitcode/Writer/ValueEnumerator.cpp   |   3 +
 llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp    |   2 +-
 llvm/lib/CodeGen/CodeGenPrepare.cpp           |   3 +
 .../lib/CodeGen/ComplexDeinterleavingPass.cpp |   3 +
 llvm/lib/IR/AsmWriter.cpp                     |   9 +-
 llvm/lib/IR/Instruction.cpp                   |   4 +-
 llvm/lib/IR/Use.cpp                           |   8 +-
 llvm/lib/IR/Value.cpp                         |  26 ++--
 .../AArch64/GISel/AArch64RegisterBankInfo.cpp |   2 +-
 llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp |  66 +++++-----
 .../InstCombineSimplifyDemanded.cpp           |   7 +-
 .../InstCombine/InstructionCombining.cpp      |   2 +-
 llvm/lib/Transforms/Scalar/Reassociate.cpp    |   3 +-
 .../test/Analysis/MemorySSA/nondeterminism.ll |   1 -
 .../llvm-diff/uselistorder-issue58629-gv.ll   |  14 +++
 .../llvm-diff/uselistorder-issue58629.ll      |   5 +-
 .../tools/llvm-reduce/bitcode-uselistorder.ll |  23 ++--
 .../uselistorder-invalid-ir-output.ll         |   6 +-
 .../verify-uselistorder.cpp                   |   9 ++
 24 files changed, 243 insertions(+), 104 deletions(-)
 create mode 100644 llvm/test/tools/llvm-diff/uselistorder-issue58629-gv.ll

diff --git a/llvm/include/llvm/IR/Use.h b/llvm/include/llvm/IR/Use.h
index a86b9c46c1f69..bcd1fd6677497 100644
--- a/llvm/include/llvm/IR/Use.h
+++ b/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
diff --git a/llvm/include/llvm/IR/Value.h b/llvm/include/llvm/IR/Value.h
index cfed12e2f5f8d..25d4750b5c0f8 100644
--- a/llvm/include/llvm/IR/Value.h
+++ b/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,51 @@ 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 +977,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 +990,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 +1029,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;
   }
diff --git a/llvm/lib/Analysis/TypeMetadataUtils.cpp b/llvm/lib/Analysis/TypeMetadataUtils.cpp
index 9ec0785eb5034..8099fbc3daeda 100644
--- a/llvm/lib/Analysis/TypeMetadataUtils.cpp
+++ b/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)) {
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index af0422f09bc4f..e806fdcdd7cad 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -8857,6 +8857,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");
 
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index 5c62ef4ad8e4e..de734aef0073e 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -3856,6 +3856,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()) {
diff --git a/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp b/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
index 9f735f77d29dc..0c81a99f2235b 100644
--- a/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
+++ b/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)
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index cf8f1c878ea5a..2fa949773ccd3 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -3953,7 +3953,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)) {
diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp
index 1bbd1b6f71b14..b728ac1c7c38a 100644
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ b/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()) {
diff --git a/llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp b/llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp
index 4cd378f9aa595..43c9c4836e207 100644
--- a/llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp
+++ b/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)
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index ac8aa0d35ea30..fedad23066084 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/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.
@@ -275,7 +279,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 =
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index 4eadecb54feb5..19dd68b3a011d 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/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) {
diff --git a/llvm/lib/IR/Use.cpp b/llvm/lib/IR/Use.cpp
index 99a89386d75f9..67882ba0144b4 100644
--- a/llvm/lib/IR/Use.cpp
+++ 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;
 }
diff --git a/llvm/lib/IR/Value.cpp b/llvm/lib/IR/Value.cpp
index 6c52ced5f73b2..025cdece7d9fb 100644
--- a/llvm/lib/IR/Value.cpp
+++ b/llvm/lib/IR/Value.cpp
@@ -53,7 +53,7 @@ static inline Type *checkType(Type *Ty) {
 Value::Value(Type *ty, unsigned scid)
     : SubclassID(scid), HasValueHandle(0), SubclassOptionalData(0),
       SubclassData(0), NumUserOperands(0), IsUsedByMD(false), HasName(false),
-      HasMetadata(false), VTy(checkType(ty)), UseList(nullptr) {
+      HasMetadata(false), VTy(checkType(ty)) {
   static_assert(ConstantFirstVal == 0, "!(SubclassID < ConstantFirstVal)");
   // FIXME: Why isn't this in the subclass gunk??
   // Note, we cannot call isa<CallInst> before the CallInst has been
@@ -147,10 +147,14 @@ void Value::destroyValueName() {
 }
 
 bool Value::hasNUses(unsigned N) const {
+  if (!hasUseList())
+    return Uses.Count == N;
   return hasNItems(use_begin(), use_end(), N);
 }
 
 bool Value::hasNUsesOrMore(unsigned N) const {
+  if (!hasUseList())
+    return Uses.Count >= N;
   return hasNItemsOrMore(use_begin(), use_end(), N);
 }
 
@@ -231,6 +235,8 @@ void Value::dropDroppableUse(Use &U) {
 }
 
 bool Value::isUsedInBasicBlock(const BasicBlock *BB) const {
+  assert(!isa<ConstantData>(this) && "ConstantData has no use-list");
+
   // This can be computed either by scanning the instructions in BB, or by
   // scanning the use list of this Value. Both lists can be very long, but
   // usually one is quite short.
@@ -252,6 +258,9 @@ bool Value::isUsedInBasicBlock(const BasicBlock *BB) const {
 }
 
 unsigned Value::getNumUses() const {
+  if (!hasUseList())
+    return Uses.Count;
+
   return (unsigned)std::distance(use_begin(), use_end());
 }
 
@@ -500,6 +509,7 @@ static bool contains(Value *Expr, Value *V) {
 #endif // NDEBUG
 
 void Value::doRAUW(Value *New, ReplaceMetadataUses ReplaceMetaUses) {
+  assert(hasUseList() && "Cannot replace constant data");
   assert(New && "Value::replaceAllUsesWith(<null>) is invalid!");
   assert(!contains(New, this) &&
          "this->replaceAllUsesWith(expr(this)) is NOT valid!");
@@ -513,7 +523,7 @@ void Value::doRAUW(Value *New, ReplaceMetadataUses ReplaceMetaUses) {
     ValueAsMetadata::handleRAUW(this, New);
 
   while (!materialized_use_empty()) {
-    Use &U = *UseList;
+    Use &U = *Uses.List;
     // Must handle Constants specially, we cannot call replaceUsesOfWith on a
     // constant because they are uniqued.
     if (auto *C = dyn_cast<Constant>(U.getUser())) {
@@ -845,7 +855,7 @@ bool Value::canBeFreed() const {
   // which is why we need the explicit opt in on a per collector basis.
   if (!F->hasGC())
     return true;
-  
+
   const auto &GCName = F->getGC();
   if (GCName == "statepoint-example") {
     auto *PT = cast<PointerType>(this->getType());
@@ -1093,12 +1103,12 @@ const Value *Value::DoPHITranslation(const BasicBlock *CurBB,
 LLVMContext &Value::getContext() const { return VTy->getContext(); }
 
 void Value::reverseUseList() {
-  if (!UseList || !UseList->Next)
+  if (!Uses.List || !Uses.List->Next || !hasUseList())
     // No need to reverse 0 or 1 uses.
     return;
 
-  Use *Head = UseList;
-  Use *Current = UseList->Next;
+  Use *Head = Uses.List;
+  Use *Current = Uses.List->Next;
   Head->Next = nullptr;
   while (Current) {
     Use *Next = Current->Next;
@@ -1107,8 +1117,8 @@ void Value::reverseUseList() {
     Head = Current;
     Current = Next;
   }
-  UseList = Head;
-  Head->Prev = &UseList;
+  Uses.List = Head;
+  Head->Prev = &Uses.List;
 }
 
 bool Value::isSwiftError() const {
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
index 8d83fef265e6f..06d7c415e070c 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
@@ -633,7 +633,7 @@ bool AArch64RegisterBankInfo::isLoadFromFPType(const MachineInstr &MI) const {
     // Look at the first element of the array to determine its type
     if (isa<ArrayType>(EltTy))
       EltTy = EltTy->getArrayElementType();
-  } else {
+  } else if (LdVal->hasUseList()) {
     // FIXME: grubbing around uses is pretty ugly, but with no more
     // `getPointerElementType` there's not much else we can do.
     for (const auto *LdUser : LdVal->users()) {
diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
index 0067d2400529a..15d80de960741 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
@@ -1104,7 +1104,7 @@ void SPIRVEmitIntrinsics::deduceOperandElementType(
   IRBuilder<> B(Ctx);
   for (auto &OpIt : Ops) {
     Value *Op = OpIt.first;
-    if (Op->use_empty())
+    if (!Op->hasUseList() || Op->use_empty())
       continue;
     if (AskOps && !AskOps->contains(Op))
       continue;
@@ -1470,34 +1470,36 @@ void SPIRVEmitIntrinsics::replacePointerOperandWithPtrCast(
   // Do not emit new spv_ptrcast if equivalent one already exists or when
   // spv_assign_ptr_type already targets this pointer with the same element
   // type.
-  for (auto User : Pointer->users()) {
-    auto *II = dyn_cast<IntrinsicInst>(User);
-    if (!II ||
-        (II->getIntrinsicID() != Intrinsic::spv_assign_ptr_type &&
-         II->getIntrinsicID() != Intrinsic::spv_ptrcast) ||
-        II->getOperand(0) != Pointer)
-      continue;
+  if (Pointer->hasUseList()) {
+    for (auto User : Pointer->users()) {
+      auto *II = dyn_cast<IntrinsicInst>(User);
+      if (!II ||
+          (II->getIntrinsicID() != Intrinsic::spv_assign_ptr_type &&
+           II->getIntrinsicID() != Intrinsic::spv_ptrcast) ||
+          II->getOperand(0) != Pointer)
+        continue;
 
-    // There is some spv_ptrcast/spv_assign_ptr_type already targeting this
-    // pointer.
-    FirstPtrCastOrAssignPtrType = false;
-    if (II->getOperand(1) != VMD ||
-        dyn_cast<ConstantInt>(II->getOperand(2))->getSExtValue() !=
-            AddressSpace)
-      continue;
+      // There is some spv_ptrcast/spv_assign_ptr_type already targeting this
+      // pointer.
+      FirstPtrCastOrAssignPtrType = false;
+      if (II->getOperand(1) != VMD ||
+          dyn_cast<ConstantInt>(II->getOperand(2))->getSExtValue() !=
+              AddressSpace)
+        continue;
 
-    // The spv_ptrcast/spv_assign_ptr_type targeting this pointer is of the same
-    // element type and address space.
-    if (II->getIntrinsicID() != Intrinsic::spv_ptrcast)
-      return;
+      // The spv_ptrcast/spv_assign_ptr_type targeting this pointer is of the
+      // same element type and address space.
+      if (II->getIntrinsicID() != Intrinsic::spv_ptrcast)
+        return;
 
-    // This must be a spv_ptrcast, do not emit new if this one has the same BB
-    // as I. Otherwise, search for other spv_ptrcast/spv_assign_ptr_type.
-    if (II->getParent() != I->getParent())
-      continue;
+      // This must be a spv_ptrcast, do not emit new if this one has the same BB
+      // as I. Otherwise, search for other spv_ptrcast/spv_assign_ptr_type.
+      if (II->getParent() != I->getParent())
+        continue;
 
-    I->setOperand(OperandToReplace, II);
-    return;
+      I->setOperand(OperandToReplace, II);
+      return;
+    }
   }
 
   if (isa<Instruction>(Pointer) || isa<Argument>(Pointer)) {
@@ -1631,6 +1633,9 @@ void SPIRVEmitIntrinsics::insertPtrCastOrAssignTypeInstr(Instruction *I,
     if (!isPointerTy(ArgOperand->getType()))
       continue;
 
+    if (!ArgOperand->hasUseList())
+      continue;
+
     // Constants (nulls/undefs) are handled in insertAssignPtrTypeIntrs()
     if (!isa<Instruction>(ArgOperand) && !isa<Argument>(ArgOperand)) {
       // However, we may have assumptions about the formal argument's type and
@@ -2487,10 +2492,13 @@ bool SPIRVEmitIntrinsics::postprocessTypes(Module &M) {
         }
       }
     }
-    for (User *U : Op->users()) {
-      Instruction *Inst = dyn_cast<Instruction>(U);
-      if (Inst && !isa<IntrinsicInst>(Inst))
-        ToProcess[Inst].insert(Op);
+
+    if (Op->hasUseList()) {
+      for (User *U : Op->users()) {
+        Instruction *Inst = dyn_cast<Instruction>(U);
+        if (Inst && !isa<IntrinsicInst>(Inst))
+          ToProcess[Inst].insert(Op);
+      }
     }
   }
   if (TodoTypeSz == 0)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
index 2c8939b5a0514..9aa75cf864446 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
@@ -1894,9 +1894,14 @@ Value *InstCombinerImpl::SimplifyDemandedVectorElts(Value *V,
         // Try to use shuffle-of-operand in place of an operand:
         // bo X, Y --> bo (shuf X), Y
         // bo X, Y --> bo X, (shuf Y)
+
+        Value *OtherOp = MatchShufAsOp0 ? Y : X;
+        if (!OtherOp->hasUseList())
+          return nullptr;
+
         BinaryOperator::BinaryOps Opcode = BO->getOpcode();
         Value *ShufOp = MatchShufAsOp0 ? X : Y;
-        Value *OtherOp = MatchShufAsOp0 ? Y : X;
+
         for (User *U : OtherOp->users()) {
           ArrayRef<int> Mask;
           auto Shuf = m_Shuffle(m_Specific(ShufOp), m_Value(), m_Mask(Mask));
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 856e02c9f1ddb..15c824a9491e4 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -1831,7 +1831,7 @@ Instruction *InstCombinerImpl::foldOpIntoPhi(Instruction &I, PHINode *PN,
     // Handle some cases that can't be fully simplified, but where we know that
     // the two instructions will fold into one.
     auto WillFold = [&]() {
-      if (!InVal->hasOneUser())
+      if (!InVal->hasUseList() || !InVal->hasOneUser())
         return false;
 
       // icmp of ucmp/scmp with constant will fold to icmp.
diff --git a/llvm/lib/Transforms/Scalar/Reassociate.cpp b/llvm/lib/Transforms/Scalar/Reassociate.cpp
index 90b891ac87a18..2b8ab24fa740b 100644
--- a/llvm/lib/Transforms/Scalar/Reassociate.cpp
+++ b/llvm/lib/Transforms/Scalar/Reassociate.cpp
@@ -437,7 +437,8 @@ static bool LinearizeExprTree(Instruction *I,
     for (unsigned OpIdx = 0; OpIdx < I->getNumOperands(); ++OpIdx) { // Visit operands.
       Value *Op = I->getOperand(OpIdx);
       LLVM_DEBUG(dbgs() << "OPERAND: " << *Op << " (" << Weight << ")\n");
-      assert(!Op->use_empty() && "No uses, so how did we get to it?!");
+      assert((isa<ConstantData>(Op) || !Op->use_empty()) &&
+             "No uses, so how did we get to it?!");
 
       // If this is a binary operation of the right kind with only one use then
       // add its operands to the expression.
diff --git a/llvm/test/Analysis/MemorySSA/nondeterminism.ll b/llvm/test/Analysis/MemorySSA/nondeterminism.ll
index 90902e36b5d58..11b9703cd0dd4 100644
--- a/llvm/test/Analysis/MemorySSA/nondeterminism.ll
+++ b/llvm/test/Analysis/MemorySSA/nondeterminism.ll
@@ -1,7 +1,6 @@
 ; RUN: opt -passes=simplifycfg -S --preserve-ll-uselistorder %s | FileCheck %s
 ; REQUIRES: x86-registered-target
 ; CHECK-LABEL: @n
-; CHECK: uselistorder i16 0, { 3, 2, 4, 1, 5, 0, 6 }
 
 ; Note: test was added in an effort to ensure determinism when updating memoryssa. See PR42574.
 ; If the uselistorder check becomes no longer relevant, the test can be disabled or removed.
diff --git a/llvm/test/tools/llvm-diff/uselistorder-issue58629-gv.ll b/llvm/test/tools/llvm-diff/uselistorder-issue58629-gv.ll
new file mode 100644
index 0000000000000..33216bb8d914e
--- /dev/null
+++ b/llvm/test/tools/llvm-diff/uselistorder-issue58629-gv.ll
@@ -0,0 +1,14 @@
+; RUN: llvm-diff %s %s | count 0
+; Make sure there is no error produced by using uselistorder with two
+; modules using the same constant/global in the same context.
+
+ at gv = addrspace(4) global [2 x i64] zeroinitializer, align 16
+
+define void @func() {
+entry:
+  %gep0 = getelementptr inbounds i8, ptr addrspace(4) @gv, i64 12
+  %gep1 = getelementptr i8, ptr addrspace(4) @gv, i64 4
+  ret void
+}
+
+uselistorder ptr addrspace(4) @gv, { 1, 0 }
diff --git a/llvm/test/tools/llvm-diff/uselistorder-issue58629.ll b/llvm/test/tools/llvm-diff/uselistorder-issue58629.ll
index e89fc7a3ea100..d50b0dcb7972d 100644
--- a/llvm/test/tools/llvm-diff/uselistorder-issue58629.ll
+++ b/llvm/test/tools/llvm-diff/uselistorder-issue58629.ll
@@ -1,5 +1,6 @@
-; XFAIL: *
-; RUN: llvm-diff %s %s
+; RUN: llvm-diff %s %s | count 0
+; Make sure there is no error produced by using uselistorder with two
+; modules using the same constant in the same context.
 
 define void @func() {
 entry:
diff --git a/llvm/test/tools/llvm-reduce/bitcode-uselistorder.ll b/llvm/test/tools/llvm-reduce/bitcode-uselistorder.ll
index ac98d75ef2d3b..4e8d1cf746441 100644
--- a/llvm/test/tools/llvm-reduce/bitcode-uselistorder.ll
+++ b/llvm/test/tools/llvm-reduce/bitcode-uselistorder.ll
@@ -11,20 +11,21 @@
 
 ; RUN: FileCheck -check-prefix=RESULT %s < %t.reduced.ll
 
+ at gv = external global i32, align 4
 
-; INTERESTING: add
-; INTERESTING: add
-; INTERESTING: add
-define i32 @func(i32 %arg0, i32 %arg1) {
+; INTERESTING: getelementptr
+; INTERESTING: getelementptr
+; INTERESTING: getelementptr
+define ptr @func(i32 %arg0, i32 %arg1, i32 %arg2, i32 %arg3, i32 %arg4) {
 entry:
-  %add0 = add i32 %arg0, 0
-  %add1 = add i32 %add0, 0
-  %add2 = add i32 %add1, 0
-  %add3 = add i32 %arg1, 0
-  %add4 = add i32 %add2, %add3
-  ret i32 %add4
+  %add0 = getelementptr i8, ptr @gv, i32 %arg0
+  %add1 = getelementptr i8, ptr @gv, i32 %arg1
+  %add2 = getelementptr i8, ptr @gv, i32 %arg2
+  %add3 = getelementptr i8, ptr @gv, i32 %arg3
+  %add4 = getelementptr i8, ptr @gv, i32 %arg4
+  ret ptr %add4
 }
 
 ; INTERESTING: uselistorder
 ; RESULT: uselistorder
-uselistorder i32 0, { 3, 2, 1, 0 }
+uselistorder ptr @gv, { 3, 2, 4, 1, 0 }
diff --git a/llvm/test/tools/llvm-reduce/uselistorder-invalid-ir-output.ll b/llvm/test/tools/llvm-reduce/uselistorder-invalid-ir-output.ll
index 4bc862bdaed26..0e9c32120f763 100644
--- a/llvm/test/tools/llvm-reduce/uselistorder-invalid-ir-output.ll
+++ b/llvm/test/tools/llvm-reduce/uselistorder-invalid-ir-output.ll
@@ -7,10 +7,11 @@
 ; RUN:   --test-arg %s
 
 ; Check if the final output really parses
-; RUN: not llvm-as -o /dev/null %t.reduced.ll
+; RUN: llvm-as -o /dev/null %t.reduced.ll
 ; RUN: FileCheck --check-prefix=RESULT %s < %t.reduced.ll
 
 
+; RESULT-LABEL: define void @kernel_ocl_path_trace_direct_lighting(
 define void @kernel_ocl_path_trace_direct_lighting(i1 %cond.i, i1 %cmp5.i.i, i32 %arg) {
 ; INTERESTING: entry:
 ; INTERESTING: 0
@@ -48,4 +49,5 @@ kernel_direct_lighting.exit:
   ret void
 }
 
-; RESULT: uselistorder i32 0, { 4, 0, 5, 1, 6, 2, 7, 3 }
+; FIXME: Should probably fix test to use a global address
+; RESULT-NOT: uselistorder
diff --git a/llvm/tools/verify-uselistorder/verify-uselistorder.cpp b/llvm/tools/verify-uselistorder/verify-uselistorder.cpp
index c2810b9579c15..4b4379713d66d 100644
--- a/llvm/tools/verify-uselistorder/verify-uselistorder.cpp
+++ b/llvm/tools/verify-uselistorder/verify-uselistorder.cpp
@@ -245,6 +245,9 @@ ValueMapping::ValueMapping(const Module &M) {
 }
 
 void ValueMapping::map(const Value *V) {
+  if (!V->hasUseList())
+    return;
+
   if (IDs.lookup(V))
     return;
 
@@ -395,6 +398,9 @@ static void verifyUseListOrder(const Module &M) {
 
 static void shuffleValueUseLists(Value *V, std::minstd_rand0 &Gen,
                                  DenseSet<Value *> &Seen) {
+  if (isa<ConstantData>(V))
+    return;
+
   if (!Seen.insert(V).second)
     return;
 
@@ -437,6 +443,9 @@ static void shuffleValueUseLists(Value *V, std::minstd_rand0 &Gen,
 }
 
 static void reverseValueUseLists(Value *V, DenseSet<Value *> &Seen) {
+  if (isa<ConstantData>(V))
+    return;
+
   if (!Seen.insert(V).second)
     return;
 



More information about the llvm-branch-commits mailing list