Skip to content

Commit 1898ab2

Browse files
allightcopybara-github
authored andcommitted
Adds support for DSLX bit vectors wider than 64 bits in the C++ transpiler.
Previously, bit vectors larger than 64 bits were incorrectly represented as `(u)int64_t`. This change introduces a new representation for wide bit vectors: `std::bitset<N>`. A new class hierarchy is introduced: - `BitVectorCppEmitter`: Base class for bit vector emitters. - `CppIntBitVectorEmitter`: Handles bit vectors up to 64 bits, using standard C++ integer types. - `RawBitVectorCppEmitter`: Handles bit vectors larger than 64 bits, using `std::bitset<N>`. Enums are a special case; their underlying type is capped at 64 bits in the C++ representation, even if the DSLX type is wider. Helper functions `FillBitSet` and `FromBitSet` are added to `xls::Bits` to facilitate conversion between `Bits` and `std::bitset<N>`. Fixes: #1135 Bug: #3764 Bug: #3763 PiperOrigin-RevId: 863265796
1 parent c44e412 commit 1898ab2

32 files changed

+208
-55
lines changed

xls/dslx/cpp_transpiler/cpp_emitter.cc

Lines changed: 124 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,10 @@ absl::StatusOr<std::string> GetBitVectorCppType(int64_t bit_count,
112112
// Type is wider that 64-bits
113113
//
114114
// This is wrong! Bit-vectors wider than 64-bits are being represented using
115-
// (u)int64_t. Fortunately this is no less wrong than the previous version of
116-
// the cpp transpiler. However, bad implementation does enable conversion of
117-
// files which contain (unused) >64-bit types though those types are
118-
// non-functional.
119-
// TODO(https://github.com/google/xls/issues/1135): Fix this.
120-
return prefix + "int64_t";
115+
// (u)int64_t.
116+
return absl::InternalError(
117+
"Attempting to create a cpp int bit-vector representation which is "
118+
"larger than 64 bits.");
121119
}
122120

123121
// Returns the number of elements in the array defined by `type_annotation`.
@@ -131,22 +129,58 @@ absl::StatusOr<int64_t> ArraySize(const ArrayTypeAnnotation* type_annotation,
131129
// types (bool, int8_t, uint16_t, etc).
132130
class BitVectorCppEmitter : public CppEmitter {
133131
public:
132+
~BitVectorCppEmitter() override = default;
133+
134+
static absl::StatusOr<std::unique_ptr<BitVectorCppEmitter>> Create(
135+
std::string_view dslx_type, int64_t dslx_bit_count, bool is_signed,
136+
bool for_enum);
137+
138+
std::string ToString(std::string_view str_to_append,
139+
std::string_view indent_amount,
140+
std::string_view identifier,
141+
int64_t nesting) const override {
142+
return absl::StrCat(str_to_append, " += \"bits[", dslx_bit_count(),
143+
"]:\" + ", ValueAsString(identifier), ";");
144+
}
145+
146+
std::string ToDslxString(std::string_view str_to_append,
147+
std::string_view indent_amount,
148+
std::string_view identifier,
149+
int64_t nesting) const override {
150+
return absl::StrCat(str_to_append, " += \"", dslx_type(), ":\" + ",
151+
ValueAsDslxString(identifier), ";");
152+
}
153+
154+
std::optional<int64_t> GetBitCountIfBitVector() const override {
155+
return dslx_bit_count_;
156+
}
157+
int64_t dslx_bit_count() const { return dslx_bit_count_; }
158+
bool is_signed() const { return is_signed_; }
159+
160+
protected:
134161
explicit BitVectorCppEmitter(const CppType& cpp_type,
135162
std::string_view dslx_type,
136163
int64_t dslx_bit_count, bool is_signed)
137164
: CppEmitter(cpp_type, dslx_type),
138165
dslx_bit_count_(dslx_bit_count),
139166
is_signed_(is_signed) {}
140-
~BitVectorCppEmitter() override = default;
167+
virtual std::string ValueAsString(std::string_view identifier) const = 0;
141168

142-
static absl::StatusOr<std::unique_ptr<BitVectorCppEmitter>> Create(
143-
std::string_view dslx_type, int64_t dslx_bit_count, bool is_signed) {
144-
XLS_ASSIGN_OR_RETURN(std::string cpp_type_name,
145-
GetBitVectorCppType(dslx_bit_count, is_signed));
146-
CppType cpp_type = {.name = cpp_type_name};
147-
return std::make_unique<BitVectorCppEmitter>(cpp_type, dslx_type,
148-
dslx_bit_count, is_signed);
149-
}
169+
virtual std::string ValueAsDslxString(std::string_view identifier) const = 0;
170+
171+
// Bit-count of the underlying DSLX type.
172+
int64_t dslx_bit_count_;
173+
bool is_signed_;
174+
};
175+
176+
// A bit vector emitter where the underlying type is a C++ int type.
177+
class CppIntBitVectorEmitter final : public BitVectorCppEmitter {
178+
public:
179+
explicit CppIntBitVectorEmitter(const CppType& cpp_type,
180+
std::string_view dslx_type,
181+
int64_t dslx_bit_count, bool is_signed)
182+
: BitVectorCppEmitter(cpp_type, dslx_type, dslx_bit_count, is_signed) {}
183+
~CppIntBitVectorEmitter() override = default;
150184

151185
std::string AssignToValue(std::string_view lhs, std::string_view rhs,
152186
int64_t nesting) const override {
@@ -216,34 +250,12 @@ class BitVectorCppEmitter : public CppEmitter {
216250
return absl::StrJoin(pieces, "\n");
217251
}
218252

219-
std::string ToString(std::string_view str_to_append,
220-
std::string_view indent_amount,
221-
std::string_view identifier,
222-
int64_t nesting) const override {
223-
return absl::StrCat(str_to_append, " += \"bits[", dslx_bit_count(),
224-
"]:\" + ", ValueAsString(identifier), ";");
225-
}
226-
227-
std::string ToDslxString(std::string_view str_to_append,
228-
std::string_view indent_amount,
229-
std::string_view identifier,
230-
int64_t nesting) const override {
231-
return absl::StrCat(str_to_append, " += \"", dslx_type(), ":\" + ",
232-
ValueAsDslxString(identifier), ";");
233-
}
234-
235-
std::optional<int64_t> GetBitCountIfBitVector() const override {
236-
return dslx_bit_count_;
237-
}
238-
int64_t dslx_bit_count() const { return dslx_bit_count_; }
239-
bool is_signed() const { return is_signed_; }
240-
241253
protected:
242-
std::string ValueAsString(std::string_view identifier) const {
254+
std::string ValueAsString(std::string_view identifier) const override {
243255
return absl::StrCat("absl::StrFormat(\"0x%x\", ", identifier, ")");
244256
}
245257

246-
std::string ValueAsDslxString(std::string_view identifier) const {
258+
std::string ValueAsDslxString(std::string_view identifier) const override {
247259
if (dslx_type() == "bool") {
248260
return absl::StrFormat("std::string{%s ? \"true\" : \"false\"}",
249261
identifier);
@@ -253,12 +265,78 @@ class BitVectorCppEmitter : public CppEmitter {
253265
}
254266
return ValueAsString(identifier);
255267
}
268+
};
256269

257-
// Bit-count of the underlying DSLX type.
258-
int64_t dslx_bit_count_;
259-
bool is_signed_;
270+
// An emitter that generates a std::bitset<size> for a bit-vector. Used if the
271+
// bit vector is larger than 64 bits.
272+
class RawBitVectorCppEmitter : public BitVectorCppEmitter {
273+
public:
274+
explicit RawBitVectorCppEmitter(const CppType& cpp_type,
275+
std::string_view dslx_type,
276+
int64_t dslx_bit_count, bool is_signed)
277+
: BitVectorCppEmitter(cpp_type, dslx_type, dslx_bit_count, is_signed) {}
278+
~RawBitVectorCppEmitter() override = default;
279+
280+
std::string AssignToValue(std::string_view lhs, std::string_view rhs,
281+
int64_t nesting) const override {
282+
std::vector<std::string> pieces;
283+
pieces.push_back(
284+
absl::StrFormat("%s = ::xls::Value(::xls::Bits::FromBitSet<%d>(%s));",
285+
lhs, dslx_bit_count(), rhs));
286+
return absl::StrJoin(pieces, "\n");
287+
}
288+
std::string AssignFromValue(std::string_view lhs, std::string_view rhs,
289+
int64_t nesting) const override {
290+
std::vector<std::string> pieces;
291+
pieces.push_back(
292+
absl::StrFormat("if (!%s.IsBits() || %s.bits().bit_count() != %d) {",
293+
rhs, rhs, dslx_bit_count()));
294+
pieces.push_back(
295+
absl::StrFormat(" return absl::InvalidArgumentError(\"Value is not a "
296+
"bits type of %d bits.\");",
297+
dslx_bit_count()));
298+
pieces.push_back("}");
299+
pieces.push_back(
300+
absl::StrFormat(" XLS_RETURN_IF_ERROR(%s.bits().FillBitSet<%d>(%s));",
301+
rhs, dslx_bit_count(), lhs));
302+
return absl::StrJoin(pieces, "\n");
303+
}
304+
std::string Verify(std::string_view identifier, std::string_view name,
305+
int64_t nesting) const override {
306+
// No need to verify anything. The array forces the right size.
307+
return "";
308+
}
309+
310+
protected:
311+
std::string ValueAsString(std::string_view identifier) const override {
312+
return absl::StrFormat(R"Fmt(absl::StrCat("0b", (%s).to_string()))Fmt",
313+
identifier);
314+
}
315+
std::string ValueAsDslxString(std::string_view identifier) const override {
316+
return ValueAsString(identifier);
317+
}
260318
};
261319

320+
absl::StatusOr<std::unique_ptr<BitVectorCppEmitter>>
321+
BitVectorCppEmitter::Create(std::string_view dslx_type, int64_t dslx_bit_count,
322+
bool is_signed, bool for_enum) {
323+
if (for_enum) {
324+
dslx_bit_count = std::min(dslx_bit_count, int64_t{64});
325+
}
326+
if (dslx_bit_count <= 64) {
327+
XLS_ASSIGN_OR_RETURN(std::string cpp_type_name,
328+
GetBitVectorCppType(dslx_bit_count, is_signed));
329+
CppType cpp_type = {.name = cpp_type_name};
330+
return std::make_unique<CppIntBitVectorEmitter>(cpp_type, dslx_type,
331+
dslx_bit_count, is_signed);
332+
} else {
333+
CppType cpp_type = {.name =
334+
absl::StrFormat("std::bitset<%d>", dslx_bit_count)};
335+
return std::make_unique<RawBitVectorCppEmitter>(cpp_type, dslx_type,
336+
dslx_bit_count, is_signed);
337+
}
338+
}
339+
262340
// An emitter for DSLX type refs. The emitted C++ code delegates all
263341
// functionality (ToString, etc) to code generated by other emitters.
264342
class TypeRefCppEmitter : public CppEmitter {
@@ -861,7 +939,7 @@ std::string DslxModuleNameToCppNamespace(std::string_view dslx_module) {
861939
/* static */ absl::StatusOr<std::unique_ptr<CppEmitter>> CppEmitter::Create(
862940
const TypeAnnotation* type_annotation, std::string_view dslx_type,
863941
TypeInfo* type_info, ImportData* import_data,
864-
std::string_view parent_namespaces) {
942+
std::string_view parent_namespaces, bool for_enum) {
865943
// Both builtin (e.g., `u32`) and array types (e.g, `sU[22]`) can represent
866944
// bit-vector types so call IsBitVectorType to identify them.
867945
std::optional<BitVectorMetadata> bit_vector_metadata =
@@ -872,9 +950,10 @@ std::string DslxModuleNameToCppNamespace(std::string_view dslx_module) {
872950
XLS_ASSIGN_OR_RETURN(int64_t bit_count,
873951
GetBitCountFromBitVectorMetadata(
874952
*bit_vector_metadata, type_info, import_data));
875-
return BitVectorCppEmitter::Create(dslx_type, bit_count,
876-
bit_vector_metadata->is_signed);
953+
return BitVectorCppEmitter::Create(
954+
dslx_type, bit_count, bit_vector_metadata->is_signed, for_enum);
877955
}
956+
XLS_RET_CHECK(!for_enum) << "Enums must be bits!";
878957
if (const ArrayTypeAnnotation* array_type =
879958
dynamic_cast<const ArrayTypeAnnotation*>(type_annotation);
880959
array_type != nullptr) {

xls/dslx/cpp_transpiler/cpp_emitter.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ class CppEmitter {
158158
static absl::StatusOr<std::unique_ptr<CppEmitter>> Create(
159159
const TypeAnnotation* type_annotation, std::string_view dslx_type,
160160
TypeInfo* type_info, ImportData* import_data,
161-
std::string_view parent_namespaces);
161+
std::string_view parent_namespaces, bool for_enum = false);
162162

163163
private:
164164
CppType cpp_type_;

xls/dslx/cpp_transpiler/cpp_transpiler.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ absl::StatusOr<CppSource> TranspileToCpp(
4444
#ifndef $0
4545
#define $0
4646
#include <array>
47+
#include <bitset>
4748
#include <cstdint>
4849
#include <ostream>
4950
#include <string>
@@ -61,6 +62,7 @@ absl::StatusOr<CppSource> TranspileToCpp(
6162
constexpr std::string_view kSourceTemplate =
6263
R"(// AUTOMATICALLY GENERATED FILE FROM `xls/dslx/cpp_transpiler`. DO NOT EDIT!
6364
#include <array>
65+
#include <bitset>
6466
#include <string>
6567
#include <string_view>
6668
#include <vector>

xls/dslx/cpp_transpiler/cpp_transpiler_test.cc

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -351,16 +351,18 @@ type MyUnsupportedSignedBit = s1;
351351
HasSubstr("Signed one-bit numbers are not supported")));
352352
}
353353

354-
TEST(CppTranspilerTest, UnsupportedTypes) {
354+
TEST(CppTranspilerTest, WideTypes) {
355355
constexpr std::string_view kModule = R"(
356-
type MyUnsupportedWideAlias = sN[123];
356+
type MyWideAlias = sN[123];
357357
358+
// NB This is not supported yet
359+
// https://github.com/google/xls/issues/1135
358360
enum MyUnsupportedWideEnum : uN[555] {
359361
A = 0,
360362
B = 1,
361363
}
362364
363-
struct MyUnsupportedWideStruct {
365+
struct MyWideStruct {
364366
wide_field: bits[100],
365367
}
366368
)";
@@ -369,16 +371,14 @@ struct MyUnsupportedWideStruct {
369371
XLS_ASSERT_OK_AND_ASSIGN(
370372
TypecheckedModule module,
371373
ParseAndTypecheck(kModule, "fake_path", "my_module", &import_data));
372-
// The types compile, but the code is wrong.
373-
// TODO(https://github.com/google/xls/issues/1135): Fix this.
374374
XLS_ASSERT_OK_AND_ASSIGN(
375375
auto result,
376376
TranspileToCpp(module.module, &import_data,
377377
/*additional_include_headers=*/{}, "/tmp/fake_path.h"));
378378
EXPECT_THAT(result.header,
379-
HasSubstr("using MyUnsupportedWideAlias = int64_t;"));
380-
EXPECT_THAT(result.header, HasSubstr("uint64_t wide_field"));
381-
EXPECT_THAT(result.header, HasSubstr("uint64_t wide_field"));
379+
HasSubstr("using MyWideAlias = std::bitset<123>;"));
380+
EXPECT_THAT(result.header, HasSubstr("std::bitset<100> wide_field"));
381+
EXPECT_THAT(result.header, HasSubstr("std::bitset<100> wide_field"));
382382
EXPECT_THAT(result.header,
383383
HasSubstr("enum class MyUnsupportedWideEnum : uint64_t"));
384384
}

xls/dslx/cpp_transpiler/cpp_type_generator.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ class EnumCppTypeGenerator : public CppTypeGenerator {
7070
XLS_ASSIGN_OR_RETURN(
7171
std::unique_ptr<CppEmitter> emitter,
7272
CppEmitter::Create(enum_def->type_annotation(), enum_def->identifier(),
73-
type_info, import_data, parent_namespaces));
73+
type_info, import_data, parent_namespaces,
74+
/*for_enum=*/true));
7475
auto generator = std::make_unique<EnumCppTypeGenerator>(
7576
DslxTypeNameToCpp(enum_def->identifier()), enum_def->identifier(),
7677
std::move(emitter));

xls/dslx/cpp_transpiler/test_types.x

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ type MyTupleAliasAlias = MyTupleAlias;
2727
type MyEmptyTuple = ();
2828
type MyArray = u17[2];
2929
type MyU5 = u5;
30+
type MyU555 = bits[555];
3031
type MyArrayOfArrays = MyU5[2][3];
3132
type MyTupleArray = MyTuple[2];
3233
type OtherTupleArray = (u32, u2)[2];

xls/dslx/cpp_transpiler/test_types_test.cc

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,28 @@ TEST(TestTypesTest, NestedStructToString) {
581581
EXPECT_EQ(test::OuterStruct::kVWidth, 7);
582582
}
583583

584+
TEST(TestTypesTest, BigBits) {
585+
test::MyU555 u555{};
586+
u555[554] = true;
587+
EXPECT_THAT(test::MyU555ToValue(u555),
588+
IsOkAndHolds(Value(Bits::MinSigned(555))));
589+
test::MyU555 from_val{};
590+
from_val[1] = true;
591+
from_val[3] = true;
592+
EXPECT_THAT(test::MyU555FromValue(Value(UBits(0b1010, 555))),
593+
IsOkAndHolds(from_val));
594+
EXPECT_EQ(
595+
test::MyU555ToString(from_val),
596+
"bits[555]:"
597+
"0b0000000000000000000000000000000000000000000000000000000000000000000000"
598+
"000000000000000000000000000000000000000000000000000000000000000000000000"
599+
"000000000000000000000000000000000000000000000000000000000000000000000000"
600+
"000000000000000000000000000000000000000000000000000000000000000000000000"
601+
"000000000000000000000000000000000000000000000000000000000000000000000000"
602+
"000000000000000000000000000000000000000000000000000000000000000000000000"
603+
"000000000000000000000000000000000000000000000000000000000000000000000000"
604+
"00000000000000000000000000000000000000000000000001010");
605+
}
584606
TEST(TestTypesTest, NestedStructEq) {
585607
test::InnerStruct a{.x = 42, .y = test::MyEnum::kB};
586608
test::InnerStruct b{.x = 42, .y = test::MyEnum::kC};

xls/dslx/cpp_transpiler/testdata/ArrayOfTyperefs.cctxt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// AUTOMATICALLY GENERATED FILE FROM `xls/dslx/cpp_transpiler`. DO NOT EDIT!
22
#include <array>
3+
#include <bitset>
34
#include <string>
45
#include <string_view>
56
#include <vector>

xls/dslx/cpp_transpiler/testdata/ArrayOfTyperefs.htxt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#ifndef TMP_FAKE_PATH_H_
33
#define TMP_FAKE_PATH_H_
44
#include <array>
5+
#include <bitset>
56
#include <cstdint>
67
#include <ostream>
78
#include <string>

xls/dslx/cpp_transpiler/testdata/BasicArray.cctxt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// AUTOMATICALLY GENERATED FILE FROM `xls/dslx/cpp_transpiler`. DO NOT EDIT!
22
#include <array>
3+
#include <bitset>
34
#include <string>
45
#include <string_view>
56
#include <vector>

0 commit comments

Comments
 (0)