[PATCH] D32508: [ValueTracking] Begin adding some useful methods to the proposed KnownBits struct

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

I definitely meant to add clearAllBits.



================
Comment at: include/llvm/Support/KnownBits.h:51
+  /// Returns true if we know the value of all bits.
+  bool isComplete() const {
+    assert(isValid() && "KnownBits conflict!");
----------------
RKSimon wrote:
>  <bikeshed>Not too sure about isComplete - bit too similar to isValid?</bikeshed>
> 
> If isComplete and getValue are likely to always be used together is it worth merging them into a single method:
> ```
> bool isCompleteValue(APInt &) ?
> ```
I made it two separate methods because it's often going to be used like

```
if (Known.isComplete())
  return ConstantInt::get(Tyep, Known.getValue());
```


================
Comment at: include/llvm/Support/KnownBits.h:53
+    assert(isValid() && "KnownBits conflict!");
+    return Zero.countPopulation() + One.countPopulation() == getBitWidth();
+  }
----------------
I'm slightly concerned that doing the population count in the single word APInt case on pre-Westmere CPUs(no harware popcnt) is worse than doing the OR and checking for all 1s. But in the multiword case the OR needs a temporary APInt so this may be better.

Considering adding something like isUnionFull to APInt to do the OR in place on each word without the allocation like intersects and isSubsetOf.


https://reviews.llvm.org/D32508