[PATCH] D32376: [ValueTracking] Introduce a KnownBits struct to wrap the two APInts for computeKnownBits

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon accepted this revision.
RKSimon added a comment.
This revision is now accepted and ready to land.

LGTM, with a few minor comments/queries.



================
Comment at: include/llvm/Support/KnownBits.h:10
+//
+// This file contains a class for reprsenting known zeros and ones used by
+// computeKnownBits.
----------------
representing


================
Comment at: lib/Analysis/ValueTracking.cpp:1399
+        Known.Zero |= Known2.Zero.reverseBits();
+        Known.One |= Known2.One.reverseBits();
         break;
----------------
Not due to this patch but shouldn't this be:
```
Known.Zero = Known2.Zero.reverseBits();
Known.One = Known2.One.reverseBits();
```


================
Comment at: lib/Analysis/ValueTracking.cpp:1404
+        Known.Zero |= Known2.Zero.byteSwap();
+        Known.One |= Known2.One.byteSwap();
         break;
----------------
Not due to this patch but shouldn't this be:
```
Known.Zero = Known2.Zero.byteSwap();
Known.One = Known2.One.byteSwap();
```


================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:179
 /// known one bits, returning them in Min/Max.
-static void computeSignedMinMaxValuesFromKnownBits(const APInt &KnownZero,
-                                                   const APInt &KnownOne,
+static void computeSignedMinMaxValuesFromKnownBits(const KnownBits &Known,
                                                    APInt &Min, APInt &Max) {
----------------
computeSignedMinMaxValuesFromKnownBits and computeUnsignedMinMaxValuesFromKnownBits might be useful inside KnowBits someday if we need them in SelectionDAG as well? Worth a TODO?


https://reviews.llvm.org/D32376