[PATCH] D128754: Refactor LLVM compression namespaces (Part 1: remove crc32)
leonardchan accepted this revision. leonardchan added a comment. This revision is now accepted and ready to land. LGTM. Should probably add `[llvm]` to the subject also. Generally it's helpful for immediately knowing which subrepo you're touching. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128754/new/ https://reviews.llvm.org/D128754 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D128754: Refactor LLVM compression namespaces (Part 1: remove crc32)
ckissane updated this revision to Diff 441541.
ckissane added a comment.
- Merge branch 'ckissane.refactor-compression.part-0' into
ckissane.refactor-compression.part-1
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128754/new/
https://reviews.llvm.org/D128754
Files:
llvm/docs/ReleaseNotes.rst
llvm/include/llvm/Support/Compression.h
llvm/lib/Support/Compression.cpp
llvm/unittests/Support/CompressionTest.cpp
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -64,12 +64,6 @@
TestZlibCompression(BinaryDataStr, zlib::DefaultCompression);
}
-TEST(CompressionTest, ZlibCRC32) {
- EXPECT_EQ(
- 0x414FA339U,
- zlib::crc32(StringRef("The quick brown fox jumps over the lazy dog")));
-}
-
#endif
}
Index: llvm/lib/Support/Compression.cpp
===
--- llvm/lib/Support/Compression.cpp
+++ llvm/lib/Support/Compression.cpp
@@ -84,10 +84,6 @@
return E;
}
-uint32_t zlib::crc32(StringRef Buffer) {
- return ::crc32(0, (const Bytef *)Buffer.data(), Buffer.size());
-}
-
#else
bool zlib::isAvailable() { return false; }
void zlib::compress(StringRef InputBuffer,
@@ -103,7 +99,4 @@
size_t UncompressedSize) {
llvm_unreachable("zlib::uncompress is unavailable");
}
-uint32_t zlib::crc32(StringRef Buffer) {
- llvm_unreachable("zlib::crc32 is unavailable");
-}
#endif
Index: llvm/include/llvm/Support/Compression.h
===
--- llvm/include/llvm/Support/Compression.h
+++ llvm/include/llvm/Support/Compression.h
@@ -42,8 +42,6 @@
SmallVectorImpl &UncompressedBuffer,
size_t UncompressedSize);
-uint32_t crc32(StringRef Buffer);
-
} // End of namespace zlib
namespace profile = llvm::compression::zlib;
Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -187,6 +187,7 @@
* Code that explictly needs ``zlib`` compression (IE zlib elf debug
sections) should use ``llvm::compression::zlib``.
* Code interfacing with compressed profile data should use
``llvm::compression::profile``.
* Code interfacing with compressed serialized data should use
``llvm::compression::serialize``.
+ * Remove crc32 from zlib compression namespace, people should use the
``llvm::crc32`` instead.
Changes to the Go bindings
--
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -64,12 +64,6 @@
TestZlibCompression(BinaryDataStr, zlib::DefaultCompression);
}
-TEST(CompressionTest, ZlibCRC32) {
- EXPECT_EQ(
- 0x414FA339U,
- zlib::crc32(StringRef("The quick brown fox jumps over the lazy dog")));
-}
-
#endif
}
Index: llvm/lib/Support/Compression.cpp
===
--- llvm/lib/Support/Compression.cpp
+++ llvm/lib/Support/Compression.cpp
@@ -84,10 +84,6 @@
return E;
}
-uint32_t zlib::crc32(StringRef Buffer) {
- return ::crc32(0, (const Bytef *)Buffer.data(), Buffer.size());
-}
-
#else
bool zlib::isAvailable() { return false; }
void zlib::compress(StringRef InputBuffer,
@@ -103,7 +99,4 @@
size_t UncompressedSize) {
llvm_unreachable("zlib::uncompress is unavailable");
}
-uint32_t zlib::crc32(StringRef Buffer) {
- llvm_unreachable("zlib::crc32 is unavailable");
-}
#endif
Index: llvm/include/llvm/Support/Compression.h
===
--- llvm/include/llvm/Support/Compression.h
+++ llvm/include/llvm/Support/Compression.h
@@ -42,8 +42,6 @@
SmallVectorImpl &UncompressedBuffer,
size_t UncompressedSize);
-uint32_t crc32(StringRef Buffer);
-
} // End of namespace zlib
namespace profile = llvm::compression::zlib;
Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -187,6 +187,7 @@
* Code that explictly needs ``zlib`` compression (IE zlib elf debug sections) should use ``llvm::compression::zlib``.
* Code interfacing with compressed profile data should use ``llvm::compression::profile``.
* Code interfacing with compressed serialized data should use ``llvm::compression::serialize``.
+ * Remove crc32 from zlib compression namespace, people should use the ``llvm::crc32`` instead.
Changes to the Go bindings
--
___
cfe-commits mailing list
cfe-commits
[PATCH] D128754: Refactor LLVM compression namespaces
ckissane updated this revision to Diff 441537.
ckissane added a comment.
make part 1 after part 0 of refactor
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128754/new/
https://reviews.llvm.org/D128754
Files:
llvm/docs/ReleaseNotes.rst
llvm/include/llvm/Support/Compression.h
llvm/lib/Support/Compression.cpp
llvm/unittests/Support/CompressionTest.cpp
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -64,12 +64,6 @@
TestZlibCompression(BinaryDataStr, zlib::DefaultCompression);
}
-TEST(CompressionTest, ZlibCRC32) {
- EXPECT_EQ(
- 0x414FA339U,
- zlib::crc32(StringRef("The quick brown fox jumps over the lazy dog")));
-}
-
#endif
}
Index: llvm/lib/Support/Compression.cpp
===
--- llvm/lib/Support/Compression.cpp
+++ llvm/lib/Support/Compression.cpp
@@ -84,10 +84,6 @@
return E;
}
-uint32_t zlib::crc32(StringRef Buffer) {
- return ::crc32(0, (const Bytef *)Buffer.data(), Buffer.size());
-}
-
#else
bool zlib::isAvailable() { return false; }
void zlib::compress(StringRef InputBuffer,
@@ -103,7 +99,4 @@
size_t UncompressedSize) {
llvm_unreachable("zlib::uncompress is unavailable");
}
-uint32_t zlib::crc32(StringRef Buffer) {
- llvm_unreachable("zlib::crc32 is unavailable");
-}
#endif
Index: llvm/include/llvm/Support/Compression.h
===
--- llvm/include/llvm/Support/Compression.h
+++ llvm/include/llvm/Support/Compression.h
@@ -42,8 +42,6 @@
SmallVectorImpl &UncompressedBuffer,
size_t UncompressedSize);
-uint32_t crc32(StringRef Buffer);
-
} // End of namespace zlib
namespace profile = llvm::compression::zlib;
Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -187,6 +187,7 @@
* Code that explictly needs ``zlib`` compression (IE zlib elf debug
sections) should use ``llvm::compression::zlib``.
* Code interfacing with compressed profile data should use
``llvm::compression::profile``.
* Code interfacing with compressed serialized data should use
``llvm::compression::serialize``.
+ * Remove crc32 from zlib compression namespace, people should use the
``llvm::crc32`` instead.
Changes to the Go bindings
--
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -64,12 +64,6 @@
TestZlibCompression(BinaryDataStr, zlib::DefaultCompression);
}
-TEST(CompressionTest, ZlibCRC32) {
- EXPECT_EQ(
- 0x414FA339U,
- zlib::crc32(StringRef("The quick brown fox jumps over the lazy dog")));
-}
-
#endif
}
Index: llvm/lib/Support/Compression.cpp
===
--- llvm/lib/Support/Compression.cpp
+++ llvm/lib/Support/Compression.cpp
@@ -84,10 +84,6 @@
return E;
}
-uint32_t zlib::crc32(StringRef Buffer) {
- return ::crc32(0, (const Bytef *)Buffer.data(), Buffer.size());
-}
-
#else
bool zlib::isAvailable() { return false; }
void zlib::compress(StringRef InputBuffer,
@@ -103,7 +99,4 @@
size_t UncompressedSize) {
llvm_unreachable("zlib::uncompress is unavailable");
}
-uint32_t zlib::crc32(StringRef Buffer) {
- llvm_unreachable("zlib::crc32 is unavailable");
-}
#endif
Index: llvm/include/llvm/Support/Compression.h
===
--- llvm/include/llvm/Support/Compression.h
+++ llvm/include/llvm/Support/Compression.h
@@ -42,8 +42,6 @@
SmallVectorImpl &UncompressedBuffer,
size_t UncompressedSize);
-uint32_t crc32(StringRef Buffer);
-
} // End of namespace zlib
namespace profile = llvm::compression::zlib;
Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -187,6 +187,7 @@
* Code that explictly needs ``zlib`` compression (IE zlib elf debug sections) should use ``llvm::compression::zlib``.
* Code interfacing with compressed profile data should use ``llvm::compression::profile``.
* Code interfacing with compressed serialized data should use ``llvm::compression::serialize``.
+ * Remove crc32 from zlib compression namespace, people should use the ``llvm::crc32`` instead.
Changes to the Go bindings
--
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listi
[PATCH] D128754: Refactor LLVM compression namespaces
MaskRay added inline comments. Comment at: llvm/docs/ReleaseNotes.rst:183-191 +* Refactor compression namespaces across the project, making way for a possible + introduction of alternatives to zlib compression in the llvm toolchain. + Changes are as follows: + * Remove crc32 from zlib compression namespace, people should use the ``llvm::crc32`` instead. + * Relocate the ``llvm::zlib`` namespace to ``llvm::compression::zlib``. + * Code that explictly needs ``zlib`` compression (IE zlib elf debug sections) should use ``llvm::compression::zlib``. + * Code interfacing with compressed profile data should use ``llvm::compression::profile``. leonardchan wrote: > I think maybe even more of these could be split into further patches. I would > expect that this patch contain just the namespace refactoring but not > necessarily any code deletion or cmake changes. I think this could be: > > - One patch that's just the namespace changes; anything else like variable > name changes or string changes would be in separate patches, then this patch > would be pure RFC and can get a quick LGTM > - One followup patch to this that adds `AlgorithmName` to the nested `zlib` > namespace. > - One patch that removes crc32 (and its test) > - One patch for cmake changes > - One patch for the `zlib_unavailable -> compression_unavailable` change and > logging string changes in profdata > > And if necessary, each of them would have an appropriate ReleaseNodes.rst > update. I agree with this plan. Changes like `zlib_unavailable` need to bring attention to the usual contributors/reviewers in the area. Comment at: llvm/lib/Support/Compression.cpp:26 -#if LLVM_ENABLE_ZLIB +using namespace compression; + Comment at: llvm/unittests/Support/CompressionTest.cpp:22 +using namespace compression; + Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128754/new/ https://reviews.llvm.org/D128754 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D128754: Refactor LLVM compression namespaces
leonardchan added inline comments. Comment at: llvm/docs/ReleaseNotes.rst:183-191 +* Refactor compression namespaces across the project, making way for a possible + introduction of alternatives to zlib compression in the llvm toolchain. + Changes are as follows: + * Remove crc32 from zlib compression namespace, people should use the ``llvm::crc32`` instead. + * Relocate the ``llvm::zlib`` namespace to ``llvm::compression::zlib``. + * Code that explictly needs ``zlib`` compression (IE zlib elf debug sections) should use ``llvm::compression::zlib``. + * Code interfacing with compressed profile data should use ``llvm::compression::profile``. I think maybe even more of these could be split into further patches. I would expect that this patch contain just the namespace refactoring but not necessarily any code deletion or cmake changes. I think this could be: - One patch that's just the namespace changes; anything else like variable name changes or string changes would be in separate patches, then this patch would be pure RFC and can get a quick LGTM - One followup patch to this that adds `AlgorithmName` to the nested `zlib` namespace. - One patch that removes crc32 (and its test) - One patch for cmake changes - One patch for the `zlib_unavailable -> compression_unavailable` change and logging string changes in profdata And if necessary, each of them would have an appropriate ReleaseNodes.rst update. Comment at: llvm/lib/Support/Compression.cpp:32 +#if LLVM_ENABLE_ZLIB + Should `createError` still be wrapped with `#if LLVM_ENABLE_ZLIB`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128754/new/ https://reviews.llvm.org/D128754 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D128754: Refactor LLVM compression namespaces
ckissane updated this revision to Diff 441113.
ckissane added a comment.
- Merge remote-tracking branch 'origin/main' into
ckissane.refactor-compression-namespace
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128754/new/
https://reviews.llvm.org/D128754
Files:
clang-tools-extra/clangd/index/Serialization.cpp
clang-tools-extra/clangd/unittests/SerializationTests.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
lld/ELF/CMakeLists.txt
lld/ELF/Driver.cpp
lld/ELF/InputSection.cpp
llvm/docs/ReleaseNotes.rst
llvm/include/llvm/ProfileData/InstrProf.h
llvm/include/llvm/ProfileData/SampleProf.h
llvm/include/llvm/Support/Compression.h
llvm/lib/MC/ELFObjectWriter.cpp
llvm/lib/ObjCopy/ELF/ELFObject.cpp
llvm/lib/Object/Decompressor.cpp
llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
llvm/lib/ProfileData/InstrProf.cpp
llvm/lib/ProfileData/SampleProf.cpp
llvm/lib/ProfileData/SampleProfReader.cpp
llvm/lib/ProfileData/SampleProfWriter.cpp
llvm/lib/Support/CMakeLists.txt
llvm/lib/Support/Compression.cpp
llvm/tools/llvm-mc/llvm-mc.cpp
llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
llvm/unittests/ProfileData/InstrProfTest.cpp
llvm/unittests/Support/CompressionTest.cpp
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -19,6 +19,8 @@
using namespace llvm;
+using namespace compression;
+
namespace {
#if LLVM_ENABLE_ZLIB
@@ -62,12 +64,6 @@
TestZlibCompression(BinaryDataStr, zlib::DefaultCompression);
}
-TEST(CompressionTest, ZlibCRC32) {
- EXPECT_EQ(
- 0x414FA339U,
- zlib::crc32(StringRef("The quick brown fox jumps over the lazy dog")));
-}
-
#endif
}
Index: llvm/unittests/ProfileData/InstrProfTest.cpp
===
--- llvm/unittests/ProfileData/InstrProfTest.cpp
+++ llvm/unittests/ProfileData/InstrProfTest.cpp
@@ -1146,17 +1146,19 @@
for (bool DoCompression : {false, true}) {
// Compressing:
std::string FuncNameStrings1;
-EXPECT_THAT_ERROR(collectPGOFuncNameStrings(
- FuncNames1, (DoCompression && zlib::isAvailable()),
- FuncNameStrings1),
- Succeeded());
+EXPECT_THAT_ERROR(
+collectPGOFuncNameStrings(
+FuncNames1, (DoCompression && compression::profile::isAvailable()),
+FuncNameStrings1),
+Succeeded());
// Compressing:
std::string FuncNameStrings2;
-EXPECT_THAT_ERROR(collectPGOFuncNameStrings(
- FuncNames2, (DoCompression && zlib::isAvailable()),
- FuncNameStrings2),
- Succeeded());
+EXPECT_THAT_ERROR(
+collectPGOFuncNameStrings(
+FuncNames2, (DoCompression && compression::profile::isAvailable()),
+FuncNameStrings2),
+Succeeded());
for (int Padding = 0; Padding < 2; Padding++) {
// Join with paddings :
Index: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
===
--- llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
+++ llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
@@ -739,7 +739,7 @@
.str()
.c_str());
}
-if (!zlib::isAvailable())
+if (!compression::zlib::isAvailable())
return createStringError(
errc::invalid_argument,
"LLVM was not compiled with LLVM_ENABLE_ZLIB: can not compress");
@@ -998,7 +998,7 @@
"--decompress-debug-sections");
}
- if (Config.DecompressDebugSections && !zlib::isAvailable())
+ if (Config.DecompressDebugSections && !compression::zlib::isAvailable())
return createStringError(
errc::invalid_argument,
"LLVM was not compiled with LLVM_ENABLE_ZLIB: cannot decompress");
Index: llvm/tools/llvm-mc/llvm-mc.cpp
===
--- llvm/tools/llvm-mc/llvm-mc.cpp
+++ llvm/tools/llvm-mc/llvm-mc.cpp
@@ -403,7 +403,7 @@
MAI->setRelaxELFRelocations(RelaxELFRel);
if (CompressDebugSections != DebugCompressionType::None) {
-if (!zlib::isAvailable()) {
+if (!compression::zlib::isAvailable()) {
WithColor::error(errs(), ProgName)
<< "build tools with zlib to enable -compress-debug-sections";
return 1;
Index: llvm/lib/Support/Compression.cpp
===
--- llvm/lib/Support/Compression.cpp
+++ llvm/lib/Support/Compression.cpp
@@ -23,11 +23,14 @@
using namespace llvm;
-#if LLVM_ENABLE_ZLIB
+using namespace compression;
+
static Error createError(StringRef Err) {
return
[PATCH] D128754: Refactor LLVM compression namespaces
ckissane updated this revision to Diff 441107.
ckissane added a comment.
- feat: refactor llvm compression namespaces
- chore: update rel notes to note zlib::crc32 remove
- feat: add AlgorithmName to compression namespaces
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128754/new/
https://reviews.llvm.org/D128754
Files:
clang-tools-extra/clangd/index/Serialization.cpp
clang-tools-extra/clangd/unittests/SerializationTests.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
lld/ELF/CMakeLists.txt
lld/ELF/Driver.cpp
lld/ELF/InputSection.cpp
llvm/docs/ReleaseNotes.rst
llvm/include/llvm/ProfileData/InstrProf.h
llvm/include/llvm/ProfileData/SampleProf.h
llvm/include/llvm/Support/Compression.h
llvm/lib/MC/ELFObjectWriter.cpp
llvm/lib/ObjCopy/ELF/ELFObject.cpp
llvm/lib/Object/Decompressor.cpp
llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
llvm/lib/ProfileData/InstrProf.cpp
llvm/lib/ProfileData/SampleProf.cpp
llvm/lib/ProfileData/SampleProfReader.cpp
llvm/lib/ProfileData/SampleProfWriter.cpp
llvm/lib/Support/CMakeLists.txt
llvm/lib/Support/Compression.cpp
llvm/tools/llvm-mc/llvm-mc.cpp
llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
llvm/unittests/ProfileData/InstrProfTest.cpp
llvm/unittests/Support/CompressionTest.cpp
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -19,6 +19,8 @@
using namespace llvm;
+using namespace compression;
+
namespace {
#if LLVM_ENABLE_ZLIB
@@ -62,12 +64,6 @@
TestZlibCompression(BinaryDataStr, zlib::DefaultCompression);
}
-TEST(CompressionTest, ZlibCRC32) {
- EXPECT_EQ(
- 0x414FA339U,
- zlib::crc32(StringRef("The quick brown fox jumps over the lazy dog")));
-}
-
#endif
}
Index: llvm/unittests/ProfileData/InstrProfTest.cpp
===
--- llvm/unittests/ProfileData/InstrProfTest.cpp
+++ llvm/unittests/ProfileData/InstrProfTest.cpp
@@ -1146,17 +1146,19 @@
for (bool DoCompression : {false, true}) {
// Compressing:
std::string FuncNameStrings1;
-EXPECT_THAT_ERROR(collectPGOFuncNameStrings(
- FuncNames1, (DoCompression && zlib::isAvailable()),
- FuncNameStrings1),
- Succeeded());
+EXPECT_THAT_ERROR(
+collectPGOFuncNameStrings(
+FuncNames1, (DoCompression && compression::profile::isAvailable()),
+FuncNameStrings1),
+Succeeded());
// Compressing:
std::string FuncNameStrings2;
-EXPECT_THAT_ERROR(collectPGOFuncNameStrings(
- FuncNames2, (DoCompression && zlib::isAvailable()),
- FuncNameStrings2),
- Succeeded());
+EXPECT_THAT_ERROR(
+collectPGOFuncNameStrings(
+FuncNames2, (DoCompression && compression::profile::isAvailable()),
+FuncNameStrings2),
+Succeeded());
for (int Padding = 0; Padding < 2; Padding++) {
// Join with paddings :
Index: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
===
--- llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
+++ llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
@@ -740,7 +740,7 @@
.str()
.c_str());
}
-if (!zlib::isAvailable())
+if (!compression::zlib::isAvailable())
return createStringError(
errc::invalid_argument,
"LLVM was not compiled with LLVM_ENABLE_ZLIB: can not compress");
@@ -999,7 +999,7 @@
"--decompress-debug-sections");
}
- if (Config.DecompressDebugSections && !zlib::isAvailable())
+ if (Config.DecompressDebugSections && !compression::zlib::isAvailable())
return createStringError(
errc::invalid_argument,
"LLVM was not compiled with LLVM_ENABLE_ZLIB: cannot decompress");
Index: llvm/tools/llvm-mc/llvm-mc.cpp
===
--- llvm/tools/llvm-mc/llvm-mc.cpp
+++ llvm/tools/llvm-mc/llvm-mc.cpp
@@ -403,7 +403,7 @@
MAI->setRelaxELFRelocations(RelaxELFRel);
if (CompressDebugSections != DebugCompressionType::None) {
-if (!zlib::isAvailable()) {
+if (!compression::zlib::isAvailable()) {
WithColor::error(errs(), ProgName)
<< "build tools with zlib to enable -compress-debug-sections";
return 1;
Index: llvm/lib/Support/Compression.cpp
===
--- llvm/lib/Support/Compression.cpp
+++ llvm/lib/Support/Compression.cpp
@@ -23,11 +23,14 @@
using namespace llvm;
-#if LLVM_ENABLE_ZLIB
+using namespace compress
[PATCH] D128754: Refactor LLVM compression namespaces
ckissane marked an inline comment as done. ckissane added a comment. Marked name of algorithm comments as done CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128754/new/ https://reviews.llvm.org/D128754 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D128754: Refactor LLVM compression namespaces
ckissane updated this revision to Diff 441101.
ckissane added a comment.
add AlgorithmName to compression namespaces
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128754/new/
https://reviews.llvm.org/D128754
Files:
clang-tools-extra/clangd/index/Serialization.cpp
clang-tools-extra/clangd/unittests/SerializationTests.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
lld/ELF/CMakeLists.txt
lld/ELF/Driver.cpp
lld/ELF/InputSection.cpp
llvm/docs/ReleaseNotes.rst
llvm/include/llvm/ProfileData/InstrProf.h
llvm/include/llvm/ProfileData/SampleProf.h
llvm/include/llvm/Support/Compression.h
llvm/lib/MC/ELFObjectWriter.cpp
llvm/lib/ObjCopy/ELF/ELFObject.cpp
llvm/lib/Object/Decompressor.cpp
llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
llvm/lib/ProfileData/InstrProf.cpp
llvm/lib/ProfileData/SampleProf.cpp
llvm/lib/ProfileData/SampleProfReader.cpp
llvm/lib/ProfileData/SampleProfWriter.cpp
llvm/lib/Support/CMakeLists.txt
llvm/lib/Support/Compression.cpp
llvm/tools/llvm-mc/llvm-mc.cpp
llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
llvm/unittests/ProfileData/InstrProfTest.cpp
llvm/unittests/Support/CompressionTest.cpp
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -19,6 +19,8 @@
using namespace llvm;
+using namespace compression;
+
namespace {
#if LLVM_ENABLE_ZLIB
@@ -62,12 +64,6 @@
TestZlibCompression(BinaryDataStr, zlib::DefaultCompression);
}
-TEST(CompressionTest, ZlibCRC32) {
- EXPECT_EQ(
- 0x414FA339U,
- zlib::crc32(StringRef("The quick brown fox jumps over the lazy dog")));
-}
-
#endif
}
Index: llvm/unittests/ProfileData/InstrProfTest.cpp
===
--- llvm/unittests/ProfileData/InstrProfTest.cpp
+++ llvm/unittests/ProfileData/InstrProfTest.cpp
@@ -1146,17 +1146,19 @@
for (bool DoCompression : {false, true}) {
// Compressing:
std::string FuncNameStrings1;
-EXPECT_THAT_ERROR(collectPGOFuncNameStrings(
- FuncNames1, (DoCompression && zlib::isAvailable()),
- FuncNameStrings1),
- Succeeded());
+EXPECT_THAT_ERROR(
+collectPGOFuncNameStrings(
+FuncNames1, (DoCompression && compression::profile::isAvailable()),
+FuncNameStrings1),
+Succeeded());
// Compressing:
std::string FuncNameStrings2;
-EXPECT_THAT_ERROR(collectPGOFuncNameStrings(
- FuncNames2, (DoCompression && zlib::isAvailable()),
- FuncNameStrings2),
- Succeeded());
+EXPECT_THAT_ERROR(
+collectPGOFuncNameStrings(
+FuncNames2, (DoCompression && compression::profile::isAvailable()),
+FuncNameStrings2),
+Succeeded());
for (int Padding = 0; Padding < 2; Padding++) {
// Join with paddings :
Index: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
===
--- llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
+++ llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
@@ -740,7 +740,7 @@
.str()
.c_str());
}
-if (!zlib::isAvailable())
+if (!compression::zlib::isAvailable())
return createStringError(
errc::invalid_argument,
"LLVM was not compiled with LLVM_ENABLE_ZLIB: can not compress");
@@ -999,7 +999,7 @@
"--decompress-debug-sections");
}
- if (Config.DecompressDebugSections && !zlib::isAvailable())
+ if (Config.DecompressDebugSections && !compression::zlib::isAvailable())
return createStringError(
errc::invalid_argument,
"LLVM was not compiled with LLVM_ENABLE_ZLIB: cannot decompress");
Index: llvm/tools/llvm-mc/llvm-mc.cpp
===
--- llvm/tools/llvm-mc/llvm-mc.cpp
+++ llvm/tools/llvm-mc/llvm-mc.cpp
@@ -403,7 +403,7 @@
MAI->setRelaxELFRelocations(RelaxELFRel);
if (CompressDebugSections != DebugCompressionType::None) {
-if (!zlib::isAvailable()) {
+if (!compression::zlib::isAvailable()) {
WithColor::error(errs(), ProgName)
<< "build tools with zlib to enable -compress-debug-sections";
return 1;
Index: llvm/lib/Support/Compression.cpp
===
--- llvm/lib/Support/Compression.cpp
+++ llvm/lib/Support/Compression.cpp
@@ -23,11 +23,14 @@
using namespace llvm;
-#if LLVM_ENABLE_ZLIB
+using namespace compression;
+
static Error createError(StringRef Err) {
return make_error(Err, inconvertibleErrorCode());
}
+#if LLVM_ENABLE_ZLIB
+
static Strin
[PATCH] D128754: Refactor LLVM compression namespaces
ckissane marked an inline comment as done. ckissane added inline comments. Comment at: clang-tools-extra/clangd/index/Serialization.cpp:242 +return error( +"Compressed string table, but compression::serialize is unavailable"); leonardchan wrote: > nit: Could we add some function that returns a string of whatever compression > is used? This way we have a more descriptive error message showing what > specifically is unavailable. Same comment elsewhere there's logging/error > reporting but the string is "compression::serialize". sure, I can add a AlgorithmName property to the zlib namespace and future compression namespaces, then we can print out compression::serialize::AlgorithmName to see what is being used CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128754/new/ https://reviews.llvm.org/D128754 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D128754: Refactor LLVM compression namespaces
ckissane marked an inline comment as done.
ckissane added inline comments.
Comment at: llvm/unittests/Support/CompressionTest.cpp:68
- 0x414FA339U,
- zlib::crc32(StringRef("The quick brown fox jumps over the lazy dog")));
-}
leonardchan wrote:
> Should this be replaced with `llvm::crc32` rather than deleting this?
No Because llvm::crc32 has it's own tests already
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128754/new/
https://reviews.llvm.org/D128754
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D128754: Refactor LLVM compression namespaces
leonardchan added a comment.
> Feedback needed on:
> this namespace approach will result in tooling compression methods being set
> at compile time, however we may want to aim for a runtime configurable
> approach in the future, likely a new revision of the compressed data formats
> that llvm interfaces with with outside tools IE:
>
> - clang ast serialization code could be changed so compressed data could have
> a flag indicating a compression type.
> - profile data code could be changed so compressed data could have a flag
> indicating a compression type as well.
Using the compile time approach for now with the runtime approach in the future
SGTM. I think some flag that could specify the decompression type is what we
want in the long term, but the compile time approach I think is ok for now to
not block you.
Comment at: clang-tools-extra/clangd/index/Serialization.cpp:242
+return error(
+"Compressed string table, but compression::serialize is unavailable");
nit: Could we add some function that returns a string of whatever compression
is used? This way we have a more descriptive error message showing what
specifically is unavailable. Same comment elsewhere there's logging/error
reporting but the string is "compression::serialize".
Comment at: llvm/unittests/Support/CompressionTest.cpp:68
- 0x414FA339U,
- zlib::crc32(StringRef("The quick brown fox jumps over the lazy dog")));
-}
Should this be replaced with `llvm::crc32` rather than deleting this?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128754/new/
https://reviews.llvm.org/D128754
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D128754: Refactor LLVM compression namespaces
ckissane updated this revision to Diff 440736.
ckissane added a comment.
up release notes
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128754/new/
https://reviews.llvm.org/D128754
Files:
clang-tools-extra/clangd/index/Serialization.cpp
clang-tools-extra/clangd/unittests/SerializationTests.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
lld/ELF/CMakeLists.txt
lld/ELF/Driver.cpp
lld/ELF/InputSection.cpp
llvm/docs/ReleaseNotes.rst
llvm/include/llvm/ProfileData/InstrProf.h
llvm/include/llvm/ProfileData/SampleProf.h
llvm/include/llvm/Support/Compression.h
llvm/lib/MC/ELFObjectWriter.cpp
llvm/lib/ObjCopy/ELF/ELFObject.cpp
llvm/lib/Object/Decompressor.cpp
llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
llvm/lib/ProfileData/InstrProf.cpp
llvm/lib/ProfileData/SampleProf.cpp
llvm/lib/ProfileData/SampleProfReader.cpp
llvm/lib/ProfileData/SampleProfWriter.cpp
llvm/lib/Support/CMakeLists.txt
llvm/lib/Support/Compression.cpp
llvm/tools/llvm-mc/llvm-mc.cpp
llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
llvm/unittests/ProfileData/InstrProfTest.cpp
llvm/unittests/Support/CompressionTest.cpp
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -19,6 +19,8 @@
using namespace llvm;
+using namespace compression;
+
namespace {
#if LLVM_ENABLE_ZLIB
@@ -62,12 +64,6 @@
TestZlibCompression(BinaryDataStr, zlib::DefaultCompression);
}
-TEST(CompressionTest, ZlibCRC32) {
- EXPECT_EQ(
- 0x414FA339U,
- zlib::crc32(StringRef("The quick brown fox jumps over the lazy dog")));
-}
-
#endif
}
Index: llvm/unittests/ProfileData/InstrProfTest.cpp
===
--- llvm/unittests/ProfileData/InstrProfTest.cpp
+++ llvm/unittests/ProfileData/InstrProfTest.cpp
@@ -1146,17 +1146,19 @@
for (bool DoCompression : {false, true}) {
// Compressing:
std::string FuncNameStrings1;
-EXPECT_THAT_ERROR(collectPGOFuncNameStrings(
- FuncNames1, (DoCompression && zlib::isAvailable()),
- FuncNameStrings1),
- Succeeded());
+EXPECT_THAT_ERROR(
+collectPGOFuncNameStrings(
+FuncNames1, (DoCompression && compression::profile::isAvailable()),
+FuncNameStrings1),
+Succeeded());
// Compressing:
std::string FuncNameStrings2;
-EXPECT_THAT_ERROR(collectPGOFuncNameStrings(
- FuncNames2, (DoCompression && zlib::isAvailable()),
- FuncNameStrings2),
- Succeeded());
+EXPECT_THAT_ERROR(
+collectPGOFuncNameStrings(
+FuncNames2, (DoCompression && compression::profile::isAvailable()),
+FuncNameStrings2),
+Succeeded());
for (int Padding = 0; Padding < 2; Padding++) {
// Join with paddings :
Index: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
===
--- llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
+++ llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
@@ -740,7 +740,7 @@
.str()
.c_str());
}
-if (!zlib::isAvailable())
+if (!compression::zlib::isAvailable())
return createStringError(
errc::invalid_argument,
"LLVM was not compiled with LLVM_ENABLE_ZLIB: can not compress");
@@ -999,7 +999,7 @@
"--decompress-debug-sections");
}
- if (Config.DecompressDebugSections && !zlib::isAvailable())
+ if (Config.DecompressDebugSections && !compression::zlib::isAvailable())
return createStringError(
errc::invalid_argument,
"LLVM was not compiled with LLVM_ENABLE_ZLIB: cannot decompress");
Index: llvm/tools/llvm-mc/llvm-mc.cpp
===
--- llvm/tools/llvm-mc/llvm-mc.cpp
+++ llvm/tools/llvm-mc/llvm-mc.cpp
@@ -403,7 +403,7 @@
MAI->setRelaxELFRelocations(RelaxELFRel);
if (CompressDebugSections != DebugCompressionType::None) {
-if (!zlib::isAvailable()) {
+if (!compression::zlib::isAvailable()) {
WithColor::error(errs(), ProgName)
<< "build tools with zlib to enable -compress-debug-sections";
return 1;
Index: llvm/lib/Support/Compression.cpp
===
--- llvm/lib/Support/Compression.cpp
+++ llvm/lib/Support/Compression.cpp
@@ -23,11 +23,14 @@
using namespace llvm;
-#if LLVM_ENABLE_ZLIB
+using namespace compression;
+
static Error createError(StringRef Err) {
return make_error(Err, inconvertibleErrorCode());
}
+#if LLVM_ENABLE_ZLIB
+
static StringRef convertZlibCodeToStrin
[PATCH] D128754: Refactor LLVM compression namespaces
ckissane created this revision.
ckissane added a reviewer: MaskRay.
ckissane added a project: LLVM.
Herald added subscribers: StephenFan, wenlei, usaxena95, kadircet, arphaman,
hiraditya, arichardson, mgorny, emaste.
Herald added a reviewer: alexander-shaposhnikov.
Herald added a reviewer: rupprecht.
Herald added a reviewer: jhenderson.
Herald added a project: All.
ckissane requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added subscribers: cfe-commits, llvm-commits.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D128754
Files:
clang-tools-extra/clangd/index/Serialization.cpp
clang-tools-extra/clangd/unittests/SerializationTests.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
lld/ELF/CMakeLists.txt
lld/ELF/Driver.cpp
lld/ELF/InputSection.cpp
llvm/docs/ReleaseNotes.rst
llvm/include/llvm/ProfileData/InstrProf.h
llvm/include/llvm/ProfileData/SampleProf.h
llvm/include/llvm/Support/Compression.h
llvm/lib/MC/ELFObjectWriter.cpp
llvm/lib/ObjCopy/ELF/ELFObject.cpp
llvm/lib/Object/Decompressor.cpp
llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
llvm/lib/ProfileData/InstrProf.cpp
llvm/lib/ProfileData/SampleProf.cpp
llvm/lib/ProfileData/SampleProfReader.cpp
llvm/lib/ProfileData/SampleProfWriter.cpp
llvm/lib/Support/CMakeLists.txt
llvm/lib/Support/Compression.cpp
llvm/tools/llvm-mc/llvm-mc.cpp
llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
llvm/unittests/ProfileData/InstrProfTest.cpp
llvm/unittests/Support/CompressionTest.cpp
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -19,6 +19,8 @@
using namespace llvm;
+using namespace compression;
+
namespace {
#if LLVM_ENABLE_ZLIB
@@ -62,12 +64,6 @@
TestZlibCompression(BinaryDataStr, zlib::DefaultCompression);
}
-TEST(CompressionTest, ZlibCRC32) {
- EXPECT_EQ(
- 0x414FA339U,
- zlib::crc32(StringRef("The quick brown fox jumps over the lazy dog")));
-}
-
#endif
}
Index: llvm/unittests/ProfileData/InstrProfTest.cpp
===
--- llvm/unittests/ProfileData/InstrProfTest.cpp
+++ llvm/unittests/ProfileData/InstrProfTest.cpp
@@ -1146,17 +1146,19 @@
for (bool DoCompression : {false, true}) {
// Compressing:
std::string FuncNameStrings1;
-EXPECT_THAT_ERROR(collectPGOFuncNameStrings(
- FuncNames1, (DoCompression && zlib::isAvailable()),
- FuncNameStrings1),
- Succeeded());
+EXPECT_THAT_ERROR(
+collectPGOFuncNameStrings(
+FuncNames1, (DoCompression && compression::profile::isAvailable()),
+FuncNameStrings1),
+Succeeded());
// Compressing:
std::string FuncNameStrings2;
-EXPECT_THAT_ERROR(collectPGOFuncNameStrings(
- FuncNames2, (DoCompression && zlib::isAvailable()),
- FuncNameStrings2),
- Succeeded());
+EXPECT_THAT_ERROR(
+collectPGOFuncNameStrings(
+FuncNames2, (DoCompression && compression::profile::isAvailable()),
+FuncNameStrings2),
+Succeeded());
for (int Padding = 0; Padding < 2; Padding++) {
// Join with paddings :
Index: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
===
--- llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
+++ llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
@@ -740,7 +740,7 @@
.str()
.c_str());
}
-if (!zlib::isAvailable())
+if (!compression::zlib::isAvailable())
return createStringError(
errc::invalid_argument,
"LLVM was not compiled with LLVM_ENABLE_ZLIB: can not compress");
@@ -999,7 +999,7 @@
"--decompress-debug-sections");
}
- if (Config.DecompressDebugSections && !zlib::isAvailable())
+ if (Config.DecompressDebugSections && !compression::zlib::isAvailable())
return createStringError(
errc::invalid_argument,
"LLVM was not compiled with LLVM_ENABLE_ZLIB: cannot decompress");
Index: llvm/tools/llvm-mc/llvm-mc.cpp
===
--- llvm/tools/llvm-mc/llvm-mc.cpp
+++ llvm/tools/llvm-mc/llvm-mc.cpp
@@ -403,7 +403,7 @@
MAI->setRelaxELFRelocations(RelaxELFRel);
if (CompressDebugSections != DebugCompressionType::None) {
-if (!zlib::isAvailable()) {
+if (!compression::zlib::isAvailable()) {
WithColor::error(errs(), ProgName)
<< "build tools with zlib to enable -compress-debug-sections";
return 1;
Index: llvm/lib/Support/Compression.cpp
=
