[PATCH] D128667: [WIP] Add Zstd ELF support

2022-07-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp:1015 + } else { +if (Config.DecompressDebugSections && !compression::zstd::isAvailable()) + return createStringError( For --decompress-debug-sections, the zstd error s

[PATCH] D128667: [WIP] Add Zstd ELF support

2022-07-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. There is kinda consensus that the proper way is to introduce `ELFCOMPRESS_ZSTD` as my proposal https://groups.google.com/g/generic-abi/c/satyPkuMisk did. This is currently blocked by an agreement from Cary Coutant that this value will be added to the generic ABI. https:/

[PATCH] D128667: [WIP] Add Zstd ELF support

2022-06-29 Thread Cole Kissane via Phabricator via cfe-commits
ckissane added a comment. In D128667#3612956 , @MaskRay wrote: > The lld/ change should be separate. And it needs tests. We should have > several tests such as mixed zlib and zstd input, malformed zstd input, > compressed level, etc. `OutputSections` sh

[PATCH] D128667: [WIP] Add Zstd ELF support

2022-06-27 Thread James Henderson via Phabricator via cfe-commits
jhenderson added inline comments. Comment at: llvm/lib/ObjCopy/ELF/ELFObject.cpp:567-568 +compression::zstd::compress( +StringRef(reinterpret_cast(OriginalData.data()), + OriginalData.size()), +CompressedData); This StringRef

[PATCH] D128667: [WIP] Add Zstd ELF support

2022-06-27 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Yeah, probably worth splitting this up a bit - somewhat hard to split up generation+parsing in LLVM itself (eg: llvm-mc+llvm-objdump), so maybe they're grouped together (alternatively the objdump support gets checked in first with precompiled/binary test files - I forg

[PATCH] D128667: [WIP] Add Zstd ELF support

2022-06-27 Thread Cole Kissane via Phabricator via cfe-commits
ckissane added a comment. In D128667#3613222 , @MaskRay wrote: > `llvm-objcopy --decompress-debug-sections clang.zstd clang.zstd.uncompress` > doesn't work. > > % /tmp/out/custom1/bin/llvm-objcopy --decompress-debug-sections clang.zstd > clang.zstd.un

[PATCH] D128667: [WIP] Add Zstd ELF support

2022-06-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. `llvm-objcopy --decompress-debug-sections clang.zstd clang.zstd.uncompress` doesn't work. % /tmp/out/custom1/bin/llvm-objcopy --decompress-debug-sections clang.zstd clang.zstd.uncompress /tmp/out/custom1/bin/llvm-objcopy: error: 'clang.zstd': '.debug_loc': zlib err

[PATCH] D128667: [WIP] Add Zstd ELF support

2022-06-27 Thread Cole Kissane via Phabricator via cfe-commits
ckissane updated this revision to Diff 440343. ckissane added a comment. improve style on ELFWriter::maybeWriteCompression fix typo in comment on zstd value for elf enum CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128667/new/ https://reviews.llvm.org/D128667 Files: clang/lib/Driver

[PATCH] D128667: [WIP] Add Zstd ELF support

2022-06-27 Thread Cole Kissane via Phabricator via cfe-commits
ckissane updated this revision to Diff 440330. ckissane added a comment. format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128667/new/ https://reviews.llvm.org/D128667 Files: clang/lib/Driver/ToolChains/Clang.cpp clang/lib/Driver/ToolChains/CommonArgs.cpp clang/lib/Driver/Tool

[PATCH] D128667: [WIP] Add Zstd ELF support

2022-06-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Tips: you may run `git diff -U0 --no-color --relative 'HEAD^' -- | path/to/llvm-project/clang/tools/clang-format/clang-format-diff.py -p1 -i` to clang-format your local change. Comment at: lld/ELF/Driver.cpp:957 +if (compression::zlib::isAvailable

[PATCH] D128667: [WIP] Add Zstd ELF support

2022-06-27 Thread Cole Kissane via Phabricator via cfe-commits
ckissane added a comment. In D128667#3612956 , @MaskRay wrote: > The lld/ change should be separate. And it needs tests. We should have > several tests such as mixed zlib and zstd input, malformed zstd input, > compressed level, etc. `OutputSections` sh

[PATCH] D128667: [WIP] Add Zstd ELF support

2022-06-27 Thread Cole Kissane via Phabricator via cfe-commits
ckissane updated this revision to Diff 440323. ckissane added a comment. add more diff context CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128667/new/ https://reviews.llvm.org/D128667 Files: clang/lib/Driver/ToolChains/Clang.cpp clang/lib/Driver/ToolChains/CommonArgs.cpp clang/

[PATCH] D128667: [WIP] Add Zstd ELF support

2022-06-27 Thread Cole Kissane via Phabricator via cfe-commits
ckissane updated this revision to Diff 440321. ckissane added a comment. remove extra struct member Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128667/new/ https://reviews.llvm.org/D128667 Files: clang/lib/Driver/ToolChains/Clang.cpp clang/l

[PATCH] D128667: [WIP] Add Zstd ELF support

2022-06-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. The lld/ change should be separate. And it needs tests. We should have several tests like mixed zlib and zstd input. OutputSections should not have a new member. I can handle the lld/ELF part. > Context not available. See https://llvm.org/docs/Phabricator.html#request

[PATCH] D128667: [WIP] Add Zstd ELF support

2022-06-27 Thread Cole Kissane via Phabricator via cfe-commits
ckissane updated this revision to Diff 440296. ckissane added a comment. simplifed codeflow Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128667/new/ https://reviews.llvm.org/D128667 Files: clang/lib/Driver/ToolChains/Clang.cpp clang/lib/Drive

[PATCH] D128667: [WIP] Add Zstd ELF support

2022-06-27 Thread Cole Kissane via Phabricator via cfe-commits
ckissane created this revision. Herald added subscribers: hiraditya, arichardson, emaste. Herald added a reviewer: alexander-shaposhnikov. Herald added a reviewer: rupprecht. Herald added a reviewer: jhenderson. Herald added a reviewer: MaskRay. Herald added a project: All. ckissane requested revie