From 88beb56d10fc0dda1f242d09833f36f3bc1eae9c Mon Sep 17 00:00:00 2001 From: Pawel Czarnecki Date: Fri, 16 Aug 2024 11:42:43 +0200 Subject: [PATCH 1/5] modules/zstd: fix formatting errors in C++ files Signed-off-by: Pawel Czarnecki --- xls/modules/zstd/data_generator.cc | 5 ++--- xls/modules/zstd/frame_header_test.cc | 8 ++++---- xls/modules/zstd/zstd_dec_test.cc | 4 ++-- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/xls/modules/zstd/data_generator.cc b/xls/modules/zstd/data_generator.cc index 81ffe95ed9..98b1eb4dba 100644 --- a/xls/modules/zstd/data_generator.cc +++ b/xls/modules/zstd/data_generator.cc @@ -60,9 +60,8 @@ static absl::StatusOr CallDecodecorpus( absl::Span args, const std::optional& cwd = std::nullopt, std::optional timeout = std::nullopt) { - XLS_ASSIGN_OR_RETURN( - std::filesystem::path path, - xls::GetXlsRunfilePath("external/zstd/decodecorpus")); + XLS_ASSIGN_OR_RETURN(std::filesystem::path path, + xls::GetXlsRunfilePath("external/zstd/decodecorpus")); std::vector cmd = {path}; cmd.insert(cmd.end(), args.begin(), args.end()); diff --git a/xls/modules/zstd/frame_header_test.cc b/xls/modules/zstd/frame_header_test.cc index e78af06577..791789eb1d 100644 --- a/xls/modules/zstd/frame_header_test.cc +++ b/xls/modules/zstd/frame_header_test.cc @@ -24,12 +24,14 @@ #include #include -#include "gtest/gtest.h" -#include "xls/common/fuzzing/fuzztest.h" #include "absl/container/flat_hash_map.h" #include "absl/types/span.h" +#include "external/zstd/lib/zstd.h" +#include "external/zstd/lib/zstd_errors.h" +#include "gtest/gtest.h" #include "xls/common/file/filesystem.h" #include "xls/common/file/get_runfile_path.h" +#include "xls/common/fuzzing/fuzztest.h" #include "xls/common/status/matchers.h" #include "xls/common/status/ret_check.h" #include "xls/dslx/create_import_data.h" @@ -42,8 +44,6 @@ #include "xls/ir/ir_test_base.h" #include "xls/ir/value.h" #include "xls/modules/zstd/data_generator.h" -#include "external/zstd/lib/zstd.h" -#include "external/zstd/lib/zstd_errors.h" namespace xls { namespace { diff --git a/xls/modules/zstd/zstd_dec_test.cc b/xls/modules/zstd/zstd_dec_test.cc index 12fbaeb8ce..a428cb8187 100644 --- a/xls/modules/zstd/zstd_dec_test.cc +++ b/xls/modules/zstd/zstd_dec_test.cc @@ -33,11 +33,12 @@ #include #include -#include "gtest/gtest.h" #include "absl/container/flat_hash_map.h" #include "absl/log/log.h" #include "absl/status/statusor.h" #include "absl/types/span.h" +#include "external/zstd/lib/zstd.h" +#include "gtest/gtest.h" #include "xls/common/file/filesystem.h" #include "xls/common/file/get_runfile_path.h" #include "xls/common/status/matchers.h" @@ -53,7 +54,6 @@ #include "xls/ir/value.h" #include "xls/jit/jit_proc_runtime.h" #include "xls/modules/zstd/data_generator.h" -#include "external/zstd/lib/zstd.h" namespace xls { namespace { From 94ebb25c5bf20973d02710326da2ec46d18ea060 Mon Sep 17 00:00:00 2001 From: Pawel Czarnecki Date: Fri, 16 Aug 2024 11:46:11 +0200 Subject: [PATCH 2/5] modules/zstd: fix formatting errors in BUILD file Signed-off-by: Pawel Czarnecki --- xls/modules/zstd/BUILD | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/xls/modules/zstd/BUILD b/xls/modules/zstd/BUILD index f26a09d45e..1ce5931703 100644 --- a/xls/modules/zstd/BUILD +++ b/xls/modules/zstd/BUILD @@ -153,16 +153,16 @@ cc_library( "@zstd//:decodecorpus", ], deps = [ + "//xls/common:subprocess", + "//xls/common/file:filesystem", + "//xls/common/file:get_runfile_path", + "//xls/common/status:status_macros", "@com_google_absl//absl/algorithm:container", "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", "@com_google_absl//absl/time", "@com_google_absl//absl/types:span", - "//xls/common:subprocess", - "//xls/common/file:filesystem", - "//xls/common/file:get_runfile_path", - "//xls/common/status:status_macros", ], ) @@ -213,8 +213,6 @@ cc_test( tags = ["manual"], deps = [ ":data_generator", - "@com_google_absl//absl/container:flat_hash_map", - "@com_google_absl//absl/types:span", "//xls/common:xls_gunit_main", "//xls/common/file:filesystem", "//xls/common/file:get_runfile_path", @@ -230,8 +228,10 @@ cc_test( "//xls/ir:bits", "//xls/ir:ir_test_base", "//xls/ir:value", - "@zstd", + "@com_google_absl//absl/container:flat_hash_map", + "@com_google_absl//absl/types:span", "@com_google_googletest//:gtest", + "@zstd", ], ) @@ -1002,10 +1002,6 @@ cc_test( tags = ["manual"], deps = [ ":data_generator", - "@com_google_absl//absl/container:flat_hash_map", - "@com_google_absl//absl/log", - "@com_google_absl//absl/status:statusor", - "@com_google_absl//absl/types:span", "//xls/common:xls_gunit_main", "//xls/common/file:filesystem", "//xls/common/file:get_runfile_path", @@ -1020,8 +1016,12 @@ cc_test( "//xls/ir:ir_parser", "//xls/ir:value", "//xls/jit:jit_proc_runtime", - "@zstd", + "@com_google_absl//absl/container:flat_hash_map", + "@com_google_absl//absl/log", + "@com_google_absl//absl/status:statusor", + "@com_google_absl//absl/types:span", "@com_google_googletest//:gtest", + "@zstd", ], ) From 99af2842e4b0deffbe785b745f6eab4e2b5c37d6 Mon Sep 17 00:00:00 2001 From: Pawel Czarnecki Date: Mon, 19 Aug 2024 14:04:28 +0200 Subject: [PATCH 3/5] modules/zstd/frame_header_test: Fix absl::Span handling Internal-tag: [#64294] Signed-off-by: Pawel Czarnecki --- xls/modules/zstd/frame_header_test.cc | 33 ++++++++++++++++++++------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/xls/modules/zstd/frame_header_test.cc b/xls/modules/zstd/frame_header_test.cc index 791789eb1d..0eb8d5dbb8 100644 --- a/xls/modules/zstd/frame_header_test.cc +++ b/xls/modules/zstd/frame_header_test.cc @@ -57,10 +57,22 @@ enum FrameHeaderStatus : uint8_t { UNSUPPORTED_WINDOW_SIZE }; -struct ZstdFrameHeader { - absl::Span kBuffer; - ZSTD_frameHeader kHeader; - size_t kResult; +class ZstdFrameHeader { + public: + absl::Span buffer() const { + return absl::MakeConstSpan(buffer_); + } + + ZSTD_frameHeader header() const { return header_; } + + size_t result() const { return result_; } + + ZstdFrameHeader(absl::Span buffer, ZSTD_frameHeader h, + size_t r) + : header_(std::move(h)), result_(r) { + std::vector v(buffer.begin(), buffer.end()); + buffer_ = v; + } // Parse a frame header from an arbitrary buffer with the ZSTD library. static absl::StatusOr Parse( absl::Span buffer) { @@ -69,8 +81,13 @@ struct ZstdFrameHeader { ZSTD_frameHeader zstd_fh; size_t result = ZSTD_getFrameHeader_advanced( &zstd_fh, buffer.data(), buffer.size(), ZSTD_f_zstd1_magicless); - return ZstdFrameHeader(buffer, zstd_fh, result); + return ZstdFrameHeader(buffer, std::move(zstd_fh), result); } + + private: + std::vector buffer_; + ZSTD_frameHeader header_; + size_t result_; }; class FrameHeaderTest : public xls::IrTestBase { @@ -113,7 +130,7 @@ class FrameHeaderTest : public xls::IrTestBase { // expected values. void RunAndExpectFrameHeader(const ZstdFrameHeader& zstd_frame_header) { // Extend buffer contents to 128 bits if necessary. - const absl::Span& buffer = zstd_frame_header.kBuffer; + const absl::Span buffer = zstd_frame_header.buffer(); std::vector buffer_extended(kDslxBufferSizeBytes, 0); absl::Span input_buffer; if (buffer.size() < kDslxBufferSizeBytes) { @@ -124,8 +141,8 @@ class FrameHeaderTest : public xls::IrTestBase { } // Decide on the expected status - ZSTD_frameHeader zstd_fh = zstd_frame_header.kHeader; - size_t result = zstd_frame_header.kResult; + ZSTD_frameHeader zstd_fh = zstd_frame_header.header(); + size_t result = zstd_frame_header.result(); FrameHeaderStatus expected_status = FrameHeaderStatus::OK; if (result != 0) { if (ZSTD_isError(result)) { From 1671da543e30fabe042855687a23c9800e5088b3 Mon Sep 17 00:00:00 2001 From: Pawel Czarnecki Date: Wed, 21 Aug 2024 09:49:05 +0200 Subject: [PATCH 4/5] modules/zstd: remove 'manual' tag from C++ test rules Internal-tag: [#64294] Signed-off-by: Pawel Czarnecki --- xls/modules/zstd/BUILD | 2 -- 1 file changed, 2 deletions(-) diff --git a/xls/modules/zstd/BUILD b/xls/modules/zstd/BUILD index 1ce5931703..fc9943071c 100644 --- a/xls/modules/zstd/BUILD +++ b/xls/modules/zstd/BUILD @@ -210,7 +210,6 @@ cc_test( ":frame_header_test_dslx", ], shard_count = 50, - tags = ["manual"], deps = [ ":data_generator", "//xls/common:xls_gunit_main", @@ -999,7 +998,6 @@ cc_test( ":zstd_dec_test.ir", ], shard_count = 50, - tags = ["manual"], deps = [ ":data_generator", "//xls/common:xls_gunit_main", From 27aea1cffd055a7869fe6ed6c77d735b6903a84e Mon Sep 17 00:00:00 2001 From: Pawel Czarnecki Date: Wed, 21 Aug 2024 11:46:07 +0200 Subject: [PATCH 5/5] modules/zstd: Remove not used absl/status dependency Internal-tag: [#64294] Signed-off-by: Pawel Czarnecki --- xls/modules/zstd/BUILD | 1 - 1 file changed, 1 deletion(-) diff --git a/xls/modules/zstd/BUILD b/xls/modules/zstd/BUILD index fc9943071c..6b72176806 100644 --- a/xls/modules/zstd/BUILD +++ b/xls/modules/zstd/BUILD @@ -158,7 +158,6 @@ cc_library( "//xls/common/file:get_runfile_path", "//xls/common/status:status_macros", "@com_google_absl//absl/algorithm:container", - "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", "@com_google_absl//absl/time",