[PATCH] D87034: [KnownBits] Implement accurate unsigned and signed max and min

Jay Foad via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 2 11:31:48 PDT 2020


foad added inline comments.


================
Comment at: llvm/unittests/Support/KnownBitsTest.cpp:103
 TEST(KnownBitsTest, BinaryExhaustive) {
   unsigned Bits = 4;
   ForeachKnownBits(Bits, [&](const KnownBits &Known1) {
----------------
lebedev.ri wrote:
> >>! In D87034#2252330, @foad wrote:
> > I have only exhaustively tested it up to 7 bits.
> 
> For one-time testing you could bump this all the way to `16`,
> but i think `7` was plenty enough to catch any correctness issues.
The time is exponential in the number of bits. 6 bits took about 1 second. 7 bits took about 20 seconds. I doubt I would make it as far as 16 bits.


================
Comment at: llvm/unittests/Support/KnownBitsTest.cpp:111-114
+      KnownBits KnownUMax(KnownAnd);
+      KnownBits KnownUMin(KnownAnd);
+      KnownBits KnownSMax(KnownAnd);
+      KnownBits KnownSMin(KnownAnd);
----------------
lebedev.ri wrote:
> i would think this should start from
> ```
>       KnownBits KnownUMax(KnownAnd) = 0;
>       KnownBits KnownUMin(KnownAnd) = -1;
>       KnownBits KnownSMax(KnownAnd) = INT_MIN;
>       KnownBits KnownSMin(KnownAnd) = INT_MAX;
> ```
No, the way these values are initialized and updated with `&=` as we discover each real result, is independent of which function we're testing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87034/new/

https://reviews.llvm.org/D87034



More information about the llvm-commits mailing list