[Support] Add KnownBits::countMaxSignedBits(). Make KnownBits::countMinSignBits() always return at least 1.

[craig.topper]

Summary

Even if we don't have any known bits, we can assume that there is
1 at least 1 sign bit. This is consistent with ComputeNumSignBits
which always returns at least.

Add KnownBits::countMaxSignedBits() which computes the number of
bits needed to represent all signed values with those known bits.

Split from D116469.

[craig.topper]

Add KnownBits::countMinSignBits()

[lebedev.ri]

This LG, thank you.

[spatel]

llvm/include/llvm/Support/KnownBits.h

256–258

The name doesn't read clearly to me. Does this seem better?

/// Returns the maximum number of bits needed to represent all possible
/// signed values with these known bits. This is the inverse of the minimum 
/// number of known sign bits. Examples for bitwidth 5:
/// 110?? --> 4
/// 0000? --> 2 
unsigned countMaxSignificantBits() const {

I thought this was what countMaxActiveBits() returns, so we should put a comment on that too to avoid confusion.
The "significant" terminology would be similar to a change in D20275.

[craig.topper]

llvm/include/llvm/Support/KnownBits.h

256–258

countMaxActiveBits() treats it as an unsigned number. It treats each 1 in a negative number as significant and ignores all leading zeros on a postive number.

I agree the name countMaxSignedBits() isn't great. It's partially derived from APInt::getMinSignedBits(). Maybe we should start the rename there and lose the "Min"?

[spatel]

llvm/include/llvm/Support/KnownBits.h

256–258

Yes, there's a lot of mismatched naming. There's similar functionality in ConstantRange too.
Starting with APInt sounds good. So we have getNumSignBits() there, and then getMinSignedBits() is derived from that.
If we rename the latter to getSignedBits(), that could easily be confused with getNumSignBits().
You know where I'm leaning. :)
"getSignificantBits()" or "getNumSignificantBits()" ?

[craig.topper]

llvm/include/llvm/Support/KnownBits.h

256–258

Added to the other renames in D116522

[foad]

llvm/include/llvm/Support/KnownBits.h

256–258

You know where I'm leaning. :)
"getSignificantBits()" or "getNumSignificantBits()" ?

I already commented in D116522 but I really don't like that we ended up with APInt::getActiveBits vs APInt::getSignificantBits for the unsigned vs signed version of the same thing.

How about getNumBitsSigned (trying to avoid confusion with "sign bits") vs getNumBitsUnsigned?

llvm/include/llvm/Support/KnownBits.h

 //===- llvm/Support/KnownBits.h - Stores known zeros/ones -------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
public:
 
   /// Returns the number of times the sign bit is replicated into the other
   /// bits.
   unsigned countMinSignBits() const {
     if (isNonNegative())
       return countMinLeadingZeros();
     if (isNegative())
       return countMinLeadingOnes();
-    return 0;
+    // Every value has at least 1 sign bit.
+    return 1;
   }
 
   /// Returns the maximum number of trailing zero bits possible.
   unsigned countMaxTrailingZeros() const {
     return One.countTrailingZeros();
   }
 
   /// Returns the maximum number of trailing one bits possible.
   unsigned countMaxTrailingOnes() const {
     return Zero.countTrailingZeros();
   }
 
   /// Returns the maximum number of leading zero bits possible.

llvm/unittests/Support/KnownBitsTest.cpp

 //===- llvm/unittest/Support/KnownBitsTest.cpp - KnownBits tests ----------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
ForeachKnownBits(Bits, [&](const KnownBits &Known) {
     unsigned Expected = 0;
     ForeachNumInKnownBits(Known, [&](const APInt &N) {
       Expected = std::max(Expected, N.getActiveBits());
     });
     EXPECT_EQ(Expected, Known.countMaxActiveBits());
   });
 }
 
+TEST(KnownBitsTest, CountMinSignedBits) {
+  const unsigned Bits = 4;
+  ForeachKnownBits(Bits, [&](const KnownBits &Known) {
+    unsigned Expected = Bits;
+    ForeachNumInKnownBits(Known, [&](const APInt &N) {
+      Expected = std::min(Expected, N.getNumSignBits());
+    });
+    EXPECT_EQ(Expected, Known.countMinSignBits());
+  });
+}
+
 TEST(KnownBitsTest, SExtOrTrunc) {
   const unsigned NarrowerSize = 4;
   const unsigned BaseSize = 6;
   const unsigned WiderSize = 8;
   APInt NegativeFitsNarrower(BaseSize, -4, /*isSigned*/ true);
   APInt NegativeDoesntFitNarrower(BaseSize, -28, /*isSigned*/ true);
   APInt PositiveFitsNarrower(BaseSize, 14);
   APInt PositiveDoesntFitNarrower(BaseSize, 36);