Merge pull request #1807 from xlsynth:cdleary/2024-12-19-flatten-to-b…

…its-c-api

PiperOrigin-RevId: 709125066

xls/codegen/block_generator_test.cc

@@ -1399,7 +1399,8 @@ endpackage : pkg
                                            }));
     BitPushBuffer buffer;
     x_value.FlattenTo(&buffer);
-    x_bits = Bits::FromBytes(buffer.GetUint8Data(), 5 * 32);
+    x_bits = Bits::FromBytes(buffer.GetUint8DataWithMsbPadding(),
+                             buffer.size_in_bits());
   }
   Bits y_bits;
   {
@@ -1412,7 +1413,8 @@ endpackage : pkg
                                            }));
     BitPushBuffer buffer;
     y_value.FlattenTo(&buffer);
-    y_bits = Bits::FromBytes(buffer.GetUint8Data(), 5 * 32);
+    y_bits = Bits::FromBytes(buffer.GetUint8DataWithMsbPadding(),
+                             buffer.size_in_bits());
   }
   Bits out_bits;
   {
@@ -1430,7 +1432,8 @@ endpackage : pkg
                                              }));
     BitPushBuffer buffer;
     out_value.FlattenTo(&buffer);
-    out_bits = Bits::FromBytes(buffer.GetUint8Data(), 10 * 32);
+    out_bits = Bits::FromBytes(buffer.GetUint8DataWithMsbPadding(),
+                               buffer.size_in_bits());
   }
 
   seq.Set("x", x_bits).Set("y", y_bits);

xls/contrib/ice40/ice40_device_rpc_strategy.cc

@@ -280,7 +280,7 @@ absl::StatusOr<Value> Ice40DeviceRpcStrategy::CallUnnamed(
     return absl::InvalidArgumentError("Cannot perform an empty-payload RPC.");
   }
 
-  std::vector<uint8_t> u8_data = buffer.GetUint8Data();
+  std::vector<uint8_t> u8_data = buffer.GetUint8DataWithLsbPadding();
 
   int64_t bytes_written = 0;
   while (bytes_written < buffer.size_in_bytes()) {

xls/data_structures/inline_bitmap.h

@@ -76,8 +76,12 @@ class InlineBitmap {
   }
 
   // Constructs a bitmap of width `bits.size()` using the given bits,
-  // interpreting index 0 as the LSD.
-  static InlineBitmap FromBits(absl::Span<bool const> bits) {
+  // interpreting index 0 as the *least* significant bit.
+  //
+  // Note: if you find you want an overload that accepts a `std::vector<bool>`,
+  // consider using an `absl::InlinedVector<bool, N>` as storage instead, as it
+  // can be converted to span.
+  static InlineBitmap FromBitsLsbIs0(absl::Span<bool const> bits) {
     InlineBitmap result(bits.size(), /*fill=*/false);
     int64_t bit_idx = 0;
     uint64_t* word = result.data_.data();
@@ -91,6 +95,15 @@ class InlineBitmap {
     return result;
   }
 
+  // As above, but index 0 of the span is the most significant bit.
+  static InlineBitmap FromBitsMsbIs0(absl::Span<bool const> bits) {
+    InlineBitmap result(bits.size(), /*fill=*/false);
+    for (int64_t i = 0; i < bits.size(); ++i) {
+      result.Set(bits.size() - i - 1, bits[i]);
+    }
+    return result;
+  }
+
   explicit InlineBitmap(int64_t bit_count, bool fill = false)
       : bit_count_(bit_count),
         data_(CeilOfRatio(bit_count, kWordBits),

xls/ir/BUILD

@@ -34,9 +34,11 @@ package(
 
 cc_library(
     name = "bit_push_buffer",
+    srcs = ["bit_push_buffer.cc"],
     hdrs = ["bit_push_buffer.h"],
     deps = [
         "//xls/common:math_util",
+        "//xls/data_structures:inline_bitmap",
     ],
 )
 
@@ -411,6 +413,7 @@ cc_test(
         "bits_test.cc",
     ],
     deps = [
+        ":bit_push_buffer",
         ":bits",
         ":bits_ops",
         ":bits_test_utils",
@@ -426,6 +429,7 @@ cc_test(
         "@com_google_absl//absl/container:inlined_vector",
         "@com_google_absl//absl/status",
         "@com_google_absl//absl/status:status_matchers",
+        "@com_google_absl//absl/strings",
         "@com_google_absl//absl/types:span",
         "@com_google_googletest//:gtest",
     ],

xls/ir/bit_push_buffer.cc

@@ -0,0 +1,66 @@
+
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "xls/ir/bit_push_buffer.h"
+
+#include <cstdint>
+#include <string>
+#include <vector>
+
+#include "xls/common/math_util.h"
+
+namespace xls {
+
+std::vector<uint8_t> BitPushBuffer::GetUint8DataWithLsbPadding() const {
+  // Implementation note: bitmap does not expose its underlying storage.
+  std::vector<uint8_t> result;
+  result.resize(CeilOfRatio(bitmap_.size(), 8UL), 0);
+  for (int64_t i = 0; i < static_cast<int64_t>(bitmap_.size()); ++i) {
+    result[i / 8] |= bitmap_[i] << (7 - i % 8);
+  }
+  return result;
+}
+
+std::vector<uint8_t> BitPushBuffer::GetUint8DataWithMsbPadding() const {
+  std::vector<uint8_t> result;
+  result.resize(CeilOfRatio(bitmap_.size(), 8UL), 0);
+  int64_t msbyte_padding_bits =
+      bitmap_.size() % 8 == 0 ? 0 : (8 - bitmap_.size() % 8);
+  int64_t msbyte_populated_bits = 8 - msbyte_padding_bits;
+  for (int64_t source_bit_index = 0;
+       source_bit_index < static_cast<int64_t>(bitmap_.size());
+       ++source_bit_index) {
+    bool bit_value = bitmap_[source_bit_index];
+    int64_t target_bit_index =
+        source_bit_index < msbyte_populated_bits
+            ? msbyte_populated_bits - source_bit_index - 1
+            : 7 - ((source_bit_index + msbyte_padding_bits) % 8);
+    int64_t target_byte_index =
+        source_bit_index < msbyte_populated_bits
+            ? 0
+            : (source_bit_index + msbyte_padding_bits) / 8;
+    result[target_byte_index] |= bit_value << target_bit_index;
+  }
+  return result;
+}
+
+std::string BitPushBuffer::ToString() const {
+  std::string result = "0b";
+  for (bool bit : bitmap_) {
+    result += bit ? '1' : '0';
+  }
+  return result;
+}
+
+}  // namespace xls

xls/ir/bit_push_buffer.h

@@ -18,7 +18,7 @@
 #include <cstdint>
 #include <vector>
 
-#include "xls/common/math_util.h"
+#include "xls/data_structures/inline_bitmap.h"
 
 namespace xls {
 
@@ -30,28 +30,50 @@ class BitPushBuffer {
   // ordering with which these pushed bits are returned in the byte sequence.
   void PushBit(bool bit) { bitmap_.push_back(bit); }
 
+  InlineBitmap ToBitmap() const {
+    return InlineBitmap::FromBitsMsbIs0(bitmap_);
+  }
+
   // Retrieves the pushed bits as a sequence of bytes.
   //
-  // The first-pushed bit goes into the MSb of the 0th byte. Concordantly, the
-  // final byte, if it is partial, will have padding zeroes in the least
-  // significant bits.
-  std::vector<uint8_t> GetUint8Data() const {
-    // Implementation note: bitmap does not expose its underlying storage.
-    std::vector<uint8_t> result;
-    result.resize(CeilOfRatio(bitmap_.size(), 8UL), 0);
-    for (int64_t i = 0; i < static_cast<int64_t>(bitmap_.size()); ++i) {
-      result[i / 8] |= bitmap_[i] << (7 - i % 8);
-    }
-    return result;
-  }
+  // The first-pushed bit goes into the MSb of the 0th byte.
+  //
+  // i.e. if we just push a single `1` bit, the byte that comes out of this
+  // function is `0x80`.
+  //
+  // The final byte, if it is partial, will have padding zeroes in the least
+  // significant bits, as shown above.
+  std::vector<uint8_t> GetUint8DataWithLsbPadding() const;
+
+  // As above, but the zero-padding is placed in the high bits of the first
+  // byte.
+  //
+  // i.e. if we just push a single `1` bit, the byte that comes out of this
+  // function is `0x01`.
+  //
+  // The first byte, if it is partial, will have padding zeroes in the most
+  // significant bits, as shown above.
+  std::vector<uint8_t> GetUint8DataWithMsbPadding() const;
 
   bool empty() const { return bitmap_.empty(); }
 
   // Returns the number of bytes required to store the currently-pushed bits.
   int64_t size_in_bytes() const { return CeilOfRatio(bitmap_.size(), 8UL); }
 
+  // Returns the number of bits currently in the buffer.
+  int64_t size_in_bits() const { return bitmap_.size(); }
+
+  // Returns a binary string representation of the currently-pushed bits; e.g.:
+  // ```c++
+  // BitPushBuffer buffer;
+  // buffer.PushBit(true);
+  // buffer.PushBit(false);
+  // buffer.ToString() == "0b10"
+  // ```
+  std::string ToString() const;
+
  private:
-  std::vector<bool> bitmap_;
+  absl::InlinedVector<bool, 64> bitmap_;
 };
 
 }  // namespace xls

xls/ir/bit_push_buffer_test.cc

@@ -30,7 +30,9 @@ TEST(BitPushBufferTest, IsEmptyAfterConstruction) {
 
   EXPECT_TRUE(buffer.empty());
   EXPECT_EQ(buffer.size_in_bytes(), 0);
-  EXPECT_THAT(buffer.GetUint8Data(), IsEmpty());
+  EXPECT_EQ(buffer.size_in_bits(), 0);
+  EXPECT_THAT(buffer.GetUint8DataWithMsbPadding(), IsEmpty());
+  EXPECT_THAT(buffer.GetUint8DataWithLsbPadding(), IsEmpty());
 }
 
 TEST(BitPushBufferTest, HasSingle0AfterPushingFalse) {
@@ -40,7 +42,9 @@ TEST(BitPushBufferTest, HasSingle0AfterPushingFalse) {
 
   EXPECT_FALSE(buffer.empty());
   EXPECT_EQ(buffer.size_in_bytes(), 1);
-  EXPECT_EQ(buffer.GetUint8Data(), std::vector<uint8_t>{0});
+  EXPECT_EQ(buffer.GetUint8DataWithMsbPadding(), std::vector<uint8_t>{0});
+  EXPECT_EQ(buffer.GetUint8DataWithLsbPadding(), std::vector<uint8_t>{0});
+  EXPECT_EQ(buffer.size_in_bits(), 1);
 }
 
 TEST(BitPushBufferTest, HasSingle1InMsbAfterPushingTrue) {
@@ -50,7 +54,9 @@ TEST(BitPushBufferTest, HasSingle1InMsbAfterPushingTrue) {
 
   EXPECT_FALSE(buffer.empty());
   EXPECT_EQ(buffer.size_in_bytes(), 1);
-  EXPECT_EQ(buffer.GetUint8Data(), std::vector<uint8_t>{1 << 7});
+  EXPECT_EQ(buffer.GetUint8DataWithLsbPadding(), std::vector<uint8_t>{1 << 7});
+  EXPECT_EQ(buffer.GetUint8DataWithMsbPadding(), std::vector<uint8_t>{1});
+  EXPECT_EQ(buffer.size_in_bits(), 1);
 }
 
 TEST(BitPushBufferTest, Has1InSecondMsbAfterPushingFalseTrue) {
@@ -61,7 +67,10 @@ TEST(BitPushBufferTest, Has1InSecondMsbAfterPushingFalseTrue) {
 
   EXPECT_FALSE(buffer.empty());
   EXPECT_EQ(buffer.size_in_bytes(), 1);
-  EXPECT_EQ(buffer.GetUint8Data(), std::vector<uint8_t>{1 << 6});
+  EXPECT_EQ(buffer.GetUint8DataWithLsbPadding(),
+            std::vector<uint8_t>{0b01 << 6});
+  EXPECT_EQ(buffer.GetUint8DataWithMsbPadding(), std::vector<uint8_t>{0b01});
+  EXPECT_EQ(buffer.size_in_bits(), 2);
 }
 
 TEST(BitPushBufferTest, IsOneByteAfterPushing8Values) {
@@ -73,6 +82,7 @@ TEST(BitPushBufferTest, IsOneByteAfterPushing8Values) {
 
   EXPECT_FALSE(buffer.empty());
   EXPECT_EQ(buffer.size_in_bytes(), 1);
+  EXPECT_EQ(buffer.size_in_bits(), 8);
 }
 
 TEST(BitPushBufferTest, Has1SecondBytesMsbAfterPushing8False1True) {
@@ -85,7 +95,17 @@ TEST(BitPushBufferTest, Has1SecondBytesMsbAfterPushing8False1True) {
 
   EXPECT_FALSE(buffer.empty());
   EXPECT_EQ(buffer.size_in_bytes(), 2);
-  EXPECT_EQ(buffer.GetUint8Data(), std::vector<uint8_t>({0, 1 << 7}));
+  EXPECT_EQ(buffer.GetUint8DataWithLsbPadding(),
+            std::vector<uint8_t>({0, 1 << 7}));
+  EXPECT_EQ(buffer.GetUint8DataWithMsbPadding(), std::vector<uint8_t>({0, 1}));
+  EXPECT_EQ(buffer.size_in_bits(), 9);
+}
+
+TEST(BitPushBufferTest, ToString) {
+  BitPushBuffer buffer;
+  buffer.PushBit(true);
+  buffer.PushBit(false);
+  EXPECT_EQ(buffer.ToString(), "0b10");
 }
 
 }  // namespace

xls/ir/bits.cc

@@ -63,7 +63,7 @@ absl::StatusOr<Bits> SBitsWithStatus(int64_t value, int64_t bit_count) {
 }
 
 Bits::Bits(absl::Span<bool const> bits)
-    : bitmap_(InlineBitmap::FromBits(bits)) {}
+    : bitmap_(InlineBitmap::FromBitsLsbIs0(bits)) {}
 
 void Bits::SetRange(int64_t start_index, int64_t end_index, bool value) {
   bitmap_.SetRange(start_index, end_index, value);

xls/ir/bits.h

@@ -249,6 +249,9 @@ class Bits {
 
 // Helper for "stringing together" bits objects into a final result, avoiding
 // intermediate allocations.
+//
+// Note that to use this object you have to know the total bit count up front.
+// If the total bit count is unknown see `BitPushBuffer`.
 class BitsRope {
  public:
   explicit BitsRope(int64_t total_bit_count) : bitmap_(total_bit_count) {}