Skip to content

Commit 5f13d7b

Browse files
lromorlromor
committed
xilinx: generic test fixes
Removed unnecessary NOLINT comments, using CHECK() instead and some other minor changes
1 parent af82033 commit 5f13d7b

File tree

7 files changed

+39
-52
lines changed

7 files changed

+39
-52
lines changed

‎fpga/xilinx/BUILD‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ cc_library(
132132
":arch-xc7-frame",
133133
"//fpga:database-parsers",
134134
"//fpga:memory-mapped-file",
135+
"@abseil-cpp//absl/container:btree",
135136
"@abseil-cpp//absl/log:check",
136137
"@abseil-cpp//absl/status:statusor",
137138
],
@@ -199,6 +200,7 @@ cc_library(
199200
":arch-xc7-configuration-packet",
200201
":bit-ops",
201202
":configuration-packet",
203+
"@abseil-cpp//absl/container:btree",
202204
"@abseil-cpp//absl/log:check",
203205
"@abseil-cpp//absl/types:optional",
204206
"@abseil-cpp//absl/types:span",
@@ -225,6 +227,7 @@ cc_test(
225227
":frames",
226228
"//fpga:database-parsers",
227229
"//fpga:memory-mapped-file",
230+
"@abseil-cpp//absl/log:check",
228231
"@abseil-cpp//absl/status:statusor",
229232
"@abseil-cpp//absl/types:optional",
230233
"@abseil-cpp//absl/types:span",
@@ -308,6 +311,7 @@ cc_library(
308311
":configuration",
309312
":frames",
310313
"//fpga:database-parsers",
314+
"@abseil-cpp//absl/container:btree",
311315
"@abseil-cpp//absl/status",
312316
"@abseil-cpp//absl/status:statusor",
313317
"@abseil-cpp//absl/strings:string_view",

‎fpga/xilinx/arch-xc7-part.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
22

33
#include <cstddef>
44
#include <cstdint>
5-
#include <map>
65
#include <optional>
76
#include <vector>
87

8+
#include "absl/container/btree_map.h"
99
#include "absl/log/check.h"
1010
#include "fpga/database-parsers.h"
1111
#include "fpga/xilinx/arch-xc7-frame.h"
@@ -25,14 +25,14 @@ static BlockType BlockTypeFrom(const fpga::ConfigBusType type) {
2525
}
2626

2727
absl::StatusOr<Row> RowFrom(const fpga::ClockRegionRow &clock_region_row) {
28-
std::map<BlockType, ConfigurationBus> row;
28+
absl::btree_map<BlockType, ConfigurationBus> row;
2929
for (const auto &bus_column_pair : clock_region_row) {
3030
const std::vector<unsigned int> &config_column = bus_column_pair.second;
3131
if (config_column.empty()) {
3232
continue;
3333
}
3434
const BlockType block_type = BlockTypeFrom(bus_column_pair.first);
35-
std::map<unsigned int, ConfigurationColumn> configuration_bus;
35+
absl::btree_map<unsigned int, ConfigurationColumn> configuration_bus;
3636
for (size_t i = 0; i < config_column.size(); ++i) {
3737
configuration_bus.emplace(i, config_column[i]);
3838
}
@@ -43,7 +43,7 @@ absl::StatusOr<Row> RowFrom(const fpga::ClockRegionRow &clock_region_row) {
4343

4444
absl::StatusOr<GlobalClockRegion> GlobalClockRegionFrom(
4545
const fpga::GlobalClockRegionHalf &half) {
46-
std::map<unsigned int, Row> rows;
46+
absl::btree_map<unsigned int, Row> rows;
4747
for (size_t i = 0; i < half.size(); ++i) {
4848
const absl::StatusOr<Row> row_status = RowFrom(half[i]);
4949
if (!row_status.ok()) {

‎fpga/xilinx/arch-xc7-part.h‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,11 @@
1313
#include <algorithm>
1414
#include <cassert>
1515
#include <cstdint>
16-
#include <map>
1716
#include <optional>
1817
#include <string>
1918
#include <utility>
2019

20+
#include "absl/container/btree_map.h"
2121
#include "absl/status/statusor.h"
2222
#include "fpga/database-parsers.h"
2323
#include "fpga/xilinx/arch-xc7-frame.h"
@@ -72,13 +72,12 @@ ConfigurationColumn::ConfigurationColumn(T first, T last) {
7272
// ConfigurationColumns.
7373
class ConfigurationBus {
7474
private:
75-
using container_type = std::map<unsigned int, ConfigurationColumn>;
75+
using container_type = absl::btree_map<unsigned int, ConfigurationColumn>;
7676

7777
public:
7878
ConfigurationBus() = default;
7979

80-
explicit ConfigurationBus(
81-
std::map<unsigned int, ConfigurationColumn> configuration_columns)
80+
explicit ConfigurationBus(container_type configuration_columns)
8281
: configuration_columns_(std::move(configuration_columns)) {}
8382
// Constructs a ConfigurationBus from iterators yielding
8483
// frame addresses. The frame address need not be contiguous or sorted
@@ -129,7 +128,7 @@ ConfigurationBus::ConfigurationBus(T first, T last) {
129128

130129
class Row {
131130
private:
132-
using container_type = std::map<BlockType, ConfigurationBus>;
131+
using container_type = absl::btree_map<BlockType, ConfigurationBus>;
133132

134133
public:
135134
Row() = default;
@@ -191,7 +190,7 @@ Row::Row(T first, T last) {
191190
// contains any number of rows, buses, and columns.
192191
class GlobalClockRegion {
193192
private:
194-
using container_type = std::map<unsigned int, Row>;
193+
using container_type = absl::btree_map<unsigned int, Row>;
195194

196195
public:
197196
GlobalClockRegion() = default;

‎fpga/xilinx/bitstream.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
#ifndef FPGA_XILINX_BITSTREAM_H
22
#define FPGA_XILINX_BITSTREAM_H
33

4-
#include <map>
54
#include <optional>
65
#include <ostream>
76
#include <string>
87

8+
#include "absl/container/btree_map.h"
99
#include "absl/status/status.h"
1010
#include "absl/status/statusor.h"
1111
#include "absl/strings/string_view.h"
@@ -35,7 +35,7 @@ class BitStream {
3535
absl::string_view part_name,
3636
absl::string_view source_name,
3737
const FramesData &frames_data, std::ostream &out) {
38-
std::map<FrameAddress, FrameWords> converted_frames;
38+
absl::btree_map<FrameAddress, FrameWords> converted_frames;
3939
for (const auto &address_words_pair : frames_data) {
4040
converted_frames.emplace(address_words_pair.first,
4141
address_words_pair.second);

‎fpga/xilinx/configuration-xc7_test.cc‎

Lines changed: 16 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include <memory>
1515
#include <vector>
1616

17+
#include "absl/log/check.h"
1718
#include "absl/status/statusor.h"
1819
#include "absl/types/optional.h"
1920
#include "absl/types/span.h"
@@ -168,15 +169,14 @@ TEST(XC7ConfigurationTest,
168169
MemoryMapFile(kTestDataBase / "xc7-configuration.debug.bit");
169170
ASSERT_TRUE(debug_bitstream_status.ok()) << debug_bitstream_status.status();
170171
auto &debug_bitstream = debug_bitstream_status.value();
171-
ASSERT_TRUE(debug_bitstream);
172+
CHECK(debug_bitstream);
172173

173174
auto debug_reader =
174175
BitstreamReader<kArch>::InitWithBytes(debug_bitstream->AsBytesView());
175-
ASSERT_TRUE(debug_reader);
176+
CHECK(debug_reader.has_value());
176177
auto debug_configuration =
177-
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
178178
Configuration<kArch>::InitWithPackets(*part, *debug_reader);
179-
ASSERT_TRUE(debug_configuration);
179+
CHECK(debug_configuration.has_value());
180180

181181
absl::StatusOr<std::unique_ptr<MemoryBlock>> perframecrc_bitstream_status =
182182
MemoryMapFile(kTestDataBase / "xc7-configuration.perframecrc.bit");
@@ -187,18 +187,14 @@ TEST(XC7ConfigurationTest,
187187

188188
auto perframecrc_reader =
189189
BitstreamReader<kArch>::InitWithBytes(perframecrc_bitstream->AsBytesView());
190-
ASSERT_TRUE(perframecrc_reader);
190+
CHECK(perframecrc_reader.has_value());
191191
auto perframecrc_configuration =
192-
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
193192
Configuration<kArch>::InitWithPackets(*part, *perframecrc_reader);
194-
ASSERT_TRUE(perframecrc_configuration);
193+
CHECK(perframecrc_configuration.has_value());
195194

196-
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
197195
for (auto debug_frame : debug_configuration->frames()) {
198196
auto perframecrc_frame =
199-
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
200197
perframecrc_configuration->frames().find(debug_frame.first);
201-
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
202198
if (perframecrc_frame == perframecrc_configuration->frames().end()) {
203199
ADD_FAILURE() << debug_frame.first
204200
<< ": missing in perframecrc bitstream";
@@ -210,12 +206,9 @@ TEST(XC7ConfigurationTest,
210206
<< debug_frame.first << ": word " << ii;
211207
}
212208
}
213-
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
214209
for (auto perframecrc_frame : perframecrc_configuration->frames()) {
215210
auto debug_frame =
216-
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
217211
debug_configuration->frames().find(perframecrc_frame.first);
218-
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
219212
if (debug_frame == debug_configuration->frames().end()) {
220213
ADD_FAILURE() << perframecrc_frame.first
221214
<< ": unexpectedly present in "
@@ -233,15 +226,14 @@ TEST(XC7ConfigurationTest, DebugAndNormalBitstreamsProduceEqualConfigurations) {
233226
MemoryMapFile(kTestDataBase / "xc7-configuration.debug.bit");
234227
ASSERT_TRUE(debug_bitstream_status.ok()) << debug_bitstream_status.status();
235228
auto &debug_bitstream = debug_bitstream_status.value();
236-
ASSERT_TRUE(debug_bitstream);
229+
CHECK(debug_bitstream);
237230

238231
auto debug_reader =
239232
BitstreamReader<kArch>::InitWithBytes(debug_bitstream->AsBytesView());
240-
ASSERT_TRUE(debug_reader);
233+
CHECK(debug_reader.has_value());
241234
auto debug_configuration =
242-
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
243235
Configuration<kArch>::InitWithPackets(*part, *debug_reader);
244-
ASSERT_TRUE(debug_configuration);
236+
CHECK(debug_configuration.has_value());
245237

246238
absl::StatusOr<std::unique_ptr<MemoryBlock>> normal_bitstream_status =
247239
MemoryMapFile(kTestDataBase / "xc7-configuration.bit");
@@ -251,16 +243,13 @@ TEST(XC7ConfigurationTest, DebugAndNormalBitstreamsProduceEqualConfigurations) {
251243

252244
auto normal_reader =
253245
BitstreamReader<kArch>::InitWithBytes(normal_bitstream->AsBytesView());
254-
ASSERT_TRUE(normal_reader);
246+
CHECK(normal_reader.has_value());
255247
auto normal_configuration =
256-
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
257248
Configuration<kArch>::InitWithPackets(*part, *normal_reader);
258-
ASSERT_TRUE(normal_configuration);
259-
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
260-
for (auto debug_frame : debug_configuration->frames()) {
261-
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
262-
auto normal_frame = normal_configuration->frames().find(debug_frame.first);
263-
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
249+
CHECK(normal_configuration.has_value());
250+
for (const auto &debug_frame : debug_configuration->frames()) {
251+
const auto normal_frame =
252+
normal_configuration->frames().find(debug_frame.first);
264253
if (normal_frame == normal_configuration->frames().end()) {
265254
ADD_FAILURE() << debug_frame.first << ": missing in normal bitstream";
266255
continue;
@@ -271,11 +260,8 @@ TEST(XC7ConfigurationTest, DebugAndNormalBitstreamsProduceEqualConfigurations) {
271260
<< debug_frame.first << ": word " << ii;
272261
}
273262
}
274-
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
275-
for (auto normal_frame : normal_configuration->frames()) {
276-
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
263+
for (const auto &normal_frame : normal_configuration->frames()) {
277264
auto debug_frame = debug_configuration->frames().find(normal_frame.first);
278-
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
279265
if (debug_frame == debug_configuration->frames().end()) {
280266
ADD_FAILURE() << normal_frame.first
281267
<< ": unexpectedly present in normal bitstream";
@@ -356,13 +342,11 @@ TEST(XC7ConfigurationTest, CheckForPaddingFrames) {
356342

357343
auto test_config =
358344
Configuration<Architecture::kXC7>::InitWithPackets(*test_part, packets);
359-
ASSERT_TRUE(test_config);
360-
// NOLINTBEGIN(bugprone-unchecked-optional-access)
345+
CHECK(test_config.has_value());
361346
ASSERT_EQ(test_config->frames().size(), 5);
362347
for (const auto &frame : test_config->frames()) {
363348
EXPECT_EQ(frame.second, frames.GetFrames().at(frame.first));
364349
}
365-
// NOLINTEND(bugprone-unchecked-optional-access)
366350
}
367351
} // namespace
368352
} // namespace xilinx

‎fpga/xilinx/configuration.h‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@
1414
#include <cstdint>
1515
#include <cstdio>
1616
#include <iterator>
17-
#include <map>
1817
#include <optional>
1918
#include <vector>
2019

20+
#include "absl/container/btree_map.h"
2121
#include "absl/types/optional.h"
2222
#include "absl/types/span.h"
2323
#include "fpga/xilinx/arch-types.h"
@@ -38,7 +38,7 @@ class Configuration {
3838
static constexpr auto kWordsPerFrame = std::tuple_size_v<FrameWords>;
3939

4040
public:
41-
using FrameMap = std::map<FrameAddress, absl::Span<const uint32_t>>;
41+
using FrameMap = absl::btree_map<FrameAddress, absl::Span<const uint32_t>>;
4242
using PacketData = std::vector<uint32_t>;
4343

4444
// Returns a configuration, i.e. collection of frame addresses
@@ -59,7 +59,7 @@ class Configuration {
5959
// Returns the payload for a type 2 packet
6060
// which allows for bigger payload compared to type 1.
6161
static PacketData CreateType2ConfigurationPacketData(
62-
const std::map<FrameAddress, FrameWords> &frames,
62+
const absl::btree_map<FrameAddress, FrameWords> &frames,
6363
std::optional<Part> &part) {
6464
PacketData packet_data;
6565
// Certain configuration frames blocks are separated by Zero Frames,
@@ -83,7 +83,8 @@ class Configuration {
8383
return packet_data;
8484
}
8585

86-
Configuration(const Part &part, std::map<FrameAddress, FrameWords> &frames)
86+
Configuration(const Part &part,
87+
absl::btree_map<FrameAddress, FrameWords> &frames)
8788
: part_(part) {
8889
for (auto &frame : frames) {
8990
frames_[frame.first] = absl::Span<const uint32_t>(frame.second);

‎fpga/xilinx/frames.h‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
#ifndef FPGA_XILINX_FRAMES_H
1111
#define FPGA_XILINX_FRAMES_H
1212

13-
#include <map>
1413
#include <optional>
1514

1615
#include "fpga/xilinx/arch-types.h"
@@ -33,18 +32,18 @@ class Frames {
3332
}
3433

3534
Frames() = default;
36-
explicit Frames(std::map<FrameAddress, FrameWords> data)
35+
explicit Frames(absl::btree_map<FrameAddress, FrameWords> data)
3736
: data_(std::move(data)) {}
3837

3938
// Adds empty frames that are present in the tilegrid of a specific part
4039
// but are missing in the current frames container.
4140
void AddMissingFrames(const std::optional<Part> &part);
4241

4342
// Returns the map with frame addresses and corresponding data
44-
std::map<FrameAddress, FrameWords> &GetFrames() { return data_; }
43+
absl::btree_map<FrameAddress, FrameWords> &GetFrames() { return data_; }
4544

4645
private:
47-
std::map<FrameAddress, FrameWords> data_;
46+
absl::btree_map<FrameAddress, FrameWords> data_;
4847

4948
// Updates the ECC information in the frame
5049
void UpdateECC(FrameWords &words);

0 commit comments

Comments
 (0)