IR: Remove null UseList checks in hasNUses methods

[arsenm]

There do not appear to be any cases where this is used.
This does introduce an odd asyemmtry where use_empty is not
equivalent to hasNUses(0).

[arsenm]

• IR: Remove null UseList checks in hasNUses methods #165929 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-llvm-ir

Author: Matt Arsenault (arsenm)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/165929.diff

1 Files Affected:

• (modified) llvm/lib/IR/Value.cpp (-8)

diff --git a/llvm/lib/IR/Value.cpp b/llvm/lib/IR/Value.cpp
index b775cbb0c7920..95d61a987f6c1 100644
--- a/llvm/lib/IR/Value.cpp
+++ b/llvm/lib/IR/Value.cpp
@@ -148,18 +148,10 @@ void Value::destroyValueName() {
 }
 
 bool Value::hasNUses(unsigned N) const {
-  if (!UseList)
-    return N == 0;
-
-  // TODO: Disallow for ConstantData and remove !UseList check?
   return hasNItems(use_begin(), use_end(), N);
 }
 
 bool Value::hasNUsesOrMore(unsigned N) const {
-  // TODO: Disallow for ConstantData and remove !UseList check?
-  if (!UseList)
-    return N == 0;
-
   return hasNItemsOrMore(use_begin(), use_end(), N);
 }
 

[nikic]

LGTM

[danilaml]

Hm, got a llvm::Value::const_use_iterator llvm::Value::materialized_use_begin() const: Assertion hasUseList()' failed.` assert on one of downstream test where

    for (Value *Op : Item->operands())
      if (Op->hasNUses(1))

Was called and I believe Op became Constant after instcombine pass. Didn't have a chance to do a closer check though so it might be something on our end.

[arsenm]

Was called and I believe Op became Constant after instcombine pass. Didn't have a chance to do a closer check though so it might be something on our end.

You shouldn't rely on the use lists of ConstantData. We still tolerate use_empty checks on them, mostly because there's just too much code to fix at a time

[danilaml]

@arsenm it wasn't relying on uselists of a constant data. It was relying on hasNUses call on Value (just instruction operands). In this case it was trivial to add a dyn_cast check but currently hasNUses feels footgun-y. All "unqualified" (i.e. on plain Value) calls are now suspect.

Also, hasOneUse still has the check, if you want to go all the way.

[arsenm]

@arsenm it wasn't relying on uselists of a constant data. It was relying on hasNUses call on Value (just instruction operands). In this case it was trivial to add a dyn_cast check but currently hasNUses feels footgun-y. All "unqualified" (i.e. on plain Value) calls are now suspect.

Also, hasOneUse still has the check, if you want to go all the way.

That's a pain. Probably should revert this then