[llvm] compression classes

[ckissane]

Summary

Adds a CompressionKind "enum" with values:
CompressionKind::Unknown ≈ 255
CompressionKind::Zlib ≈ 1
CompressionKind::ZStd ≈ 2

also note: OptionalCompressionKind is typedef'ed as Optional<CompressionKind>, and NoneType() is used to indicate no compression.

The CompressionKind "enum" has overrides for several operators:

CompressionKind::operator uint8_t() const
(Self-explanatory)

CompressionKind::operator bool() const
(true if supported, false otherwise)

bool operator==(CompressionKind a, CompressionKind b)

(Self-explanatory)

none compression is represented simply as a none type for use cases that will use Optional<CompressionKind>.
once you have a CompressionKind itself you can pass it around as a value (because a CompressionKind is just a struct containing one uint8_t (a fake "enum")).
due to my operator overloading, you can do stuff like this:

llvm::compression::OptionalCompressionKind OptionalCompressionScheme =
          llvm::compression::getOptionalCompressionKind(CompressionSchemeId);
      if (!OptionalCompressionScheme) {
        return llvm::MemoryBuffer::getMemBuffer(Blob, Name, true);
      }
      llvm::compression::CompressionKind CompressionScheme =
          *OptionalCompressionScheme;
      if (!CompressionScheme) {
        Error("compression class " +
              (CompressionScheme->Name + " is not available").str());
        return nullptr;
      }
      SmallVector<uint8_t, 0> Uncompressed;
      if (llvm::Error E = CompressionScheme->decompress(
              llvm::arrayRefFromStringRef(Blob), Uncompressed, Record[0])) {
        Error("could not decompress embedded file contents: " +
              llvm::toString(std::move(E)));
        return nullptr;
      }
      return llvm::MemoryBuffer::getMemBufferCopy(
          llvm::toStringRef(Uncompressed), Name);

(excerpt from ASTReader.cpp)

you can even do calls like

llvm::compression::CompressionKind::Zlib->decompress(...)
(this can be useful in cases like elf decompression where you might switch on debug section type and in a switch case for DebugCompressionType::Z)

I also believe similar semantics to the nullptr suggestion have been achieved, due to the bool cast returning supported status, and the Unknown type acting as a nullptr of sorts, being always unsupported and -1 (255) as a uint8. Optional(Unknown) is returned from getOptionalCompressionKind(uint8_t) when it is not 0 (NoneType()), 1 (Optional(Zlib)), or 2 (Optional(ZStd)).

In places where explict compression must be used CompressionKind can be passed around, and possibly optional (sometimes none) compression is represented as llvm::Optional<CompressionKind> which I have type aliased as OptionalCompressionKind

[ckissane]

• format
• merge fix

[dblaikie]

clang/lib/Serialization/ASTWriter.cpp

2003–2004

Doesn't this cause slicing & end up with the base implementation?

(also the base class CompressionAlgorithm has no virtual functions, so I'm not sure how this is meant to work - does this code all work? Then I must be missing some things - how does this work?)

[MaskRay]

Thanks for experimenting the refactoring. My gut feeling is that

• for inheritance llvm/lib/Support/Compression.cpp introduces quite a bit of complexity.
• BestSpeedCompression/DefaultCompression/BestSizeCompression may be kinda weird. Not all algorithms may need all of three.
• this new interface does not make parallel compression/decompression easier.

I mentioned this on https://groups.google.com/g/generic-abi/c/satyPkuMisk/m/xRqMj8M3AwAJ

I know the paradox of choice:) Certainly that we should not add a plethora of bzip2, xz, lzo, brotli, etc to generic-abi. I don't think users are so fond of using a different format for a slight different read/write/compression ratio/memory usage need. They want to pick one format which performs well across a wide variety of workloads. (Adding a new format also introduces complexity on their build system side. They need to teach DWARF consumers they use. It's not a small undertaking.)

I think for a long time llvm/lib/Support/Compression.cpp will stay with just zlib and zstd. There is a question whether we want to make the heavier abstraction now. There is a possibility if a llvm-project use case needs an alternative (it needs very strong arguments not using zstd), it has the other approach that not using llvm/Support/Compression.h

[ckissane]

clang/lib/Serialization/ASTWriter.cpp

2003–2004

You are correct to observe that this patch does not fully pass around pointers to instances of the classes, however, because I don't pass pointers and the currently repetitive nature of the compression classes, this still functions correctly.
In short, a follow-up patch (which I will shortly upload) will convert this to using class instances and passing those around.
Including reworking functions throughout llvm-project to take advantage of this.
I am aiming to take this 2 step process to cut down on making an already large pass larger.
Let me know if you have any concerns or ideas.

[dblaikie]

clang/lib/Serialization/ASTWriter.cpp

2003–2004

But I'm not sure how this patch works correctly - wouldn't the line below (CompressionScheme.supported()) call CompressionAlgorithm::supported() which always returns false?

[ckissane]

clang/lib/Serialization/ASTWriter.cpp

2003–2004

good catch

[ckissane]

• feat: compression class + clang ast serial zstd option + zstd elf + ztsd objcopy support
• [MC] fix merge issues with removal of GNU compression
• fix compression class inheritence and passing
• update usages of -enable-name-compression=false to be -name-compression=none
• do not exit on parse of bad compression cl::opt

[ckissane]

• fix compression class usage in some profile data tests

[dblaikie]

Any chance this could be split up to be more directly comparable to https://reviews.llvm.org/D130458 ?

clang-tools-extra/clangd/index/Serialization.cpp

32

We're generally trying to avoid global ctors in LLVM. So at most this should be a static local variable in a function that accesses the algorithm (though perhaps these "compression algorithm" classes shouldn't be accessible directly, and only through singleton accessors in the defined alongside - like there's no reason for LLVM to contain more than one instance of ZlibCompressionAlgorithm, I think?)

llvm/include/llvm/Support/Compression.h

71

Rather than supported() maybe the accessor functions could return nullptr when support isn't available?

if (CompressionAlgorithm *A = getZstdCompressionScheme())

etc.

Though I guess that doesn't allow for a default implementation - I guess an alternative function could be CompressionAlgorithm& getCompressionSchemeOrNone(Zstd) which always gives a valid CompressionAlgorithm by giving the do-nothing compression algorithm when the specified one is not available.

But I guess we don't generally want to silently fallback to null compression, because the streams we're producing always need to know if they have to emit headers, etc, or not? So maybe there's no need for a default?

[ckissane]

• tiny cleanup of using NoneCompressionAlgorithm::AlgorithmId

[ckissane]

In D130516#3683384, @dblaikie wrote:

Any chance this could be split up to be more directly comparable to https://reviews.llvm.org/D130458 ?

yes definitely! Doing so now!

[ckissane]

• fix compression inheritence and some more compression class helpers

[ckissane]

llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp

54

note the helpers such as when(bool), whensupported() and notNone()

[ckissane]

• use CompressionScheme->notNone() in InstrProf

[ckissane]

clang-tools-extra/clangd/index/Serialization.cpp

32

your idea seems correct to me, however
some algorithms, such as zstd, support running in multiple threads, so this might influence our decision.

[dblaikie]

clang-tools-extra/clangd/index/Serialization.cpp

32

So long as the compression algorithm objects are stateless, this would be acceptable - such objects would be thread safe for multiple concurrent users.

[ckissane]

marked outdated comments as done

[dblaikie]

llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp

50–54

This seems a bit too convoluted for me.

I'd think something like:

if (DoInstrProfNameCompression) {
  if (CompressionAlgorithm *C = getZlibCompressionAlgorithm())
    C->compress(...);
}

Or even have getCompressionAlgorithm(SupportCompressionType::Zlib) (like that could be the only entry point - no need for algorithm-specific accessors, that function would have one switch over SupportCompressionType, returning null if Unknown or Null were passed, or if the requested algorithm was not available)

I'm not sure I understand the 'when'/'whenSupported' stuff and whether there's any value/need for more details to be communicated in the not-available case other than 'false'/null/nothing (like, if it needs to communicate a reason for non-availability, that's more involved than returning null from some factory/accessor function).

llvm/lib/Support/Compression.cpp

98–102

Maybe these don't need to be static members - if there are singleton insntances of the algorithms, they could be members of those singletons instead (possibly in the base/impl class - the derived classes could pass these values into the base ctor to initialize members in the impl or base - they could even be const public members, avoid the need for accessors (at least avoiding the need for virtual accessors, but hopefully avoiding accessors entirely))

[leonardchan]

clang-tools-extra/clangd/index/Serialization.cpp

193–196

Will this leak?

llvm/include/llvm/Support/Compression.h

56–58

Does the uncompress version of this just end up calling into the other uncompress function? If so, we could probably just have one decompress virtual method here and the one that accepts a SmallVectorImpl just calls into the virtual decompress rather than have two virtual methods that would do the same thing. It looks like you've done that in CompressionAlgorithmImpl, but I think it could be moved here.

60–61

Perhaps add some comments for these functions? At least for me, it's not entirely clear what these are for.

66–67

Perhaps it would be simpler to just have the individual subclasses inherit from CompressionAlgorithm rather than have them all go through CompressionAlgorithmImpl? It looks like each child class with methods like getAlgorithmId can just return the static values themselves rather than passing them up to a parent to be returned. I think unless some static polymorphism is needed here, CRTP might not be needed here.

llvm/lib/Support/Compression.cpp

68

Does NoneCompressionAlgorithm imply there's no compression at all? If so, I would think these methods should be empty.

227–228

Is the purpose of UnknownCompressionAlgorithm to be the default instance here? If so, would it be better perhaps to just omit this and have an llvm_unreachable in the default case below? I would assume users of this function should just have the right compression scheme ID they need and any error checking on if something is a valid ID would be done before calling this.

[ckissane]

• make compression singletons

[ckissane]

• fix usage of CompressionAlgorithmFromId

[MaskRay]

I'd like to make a few arguments for the current namespace+free function design, as opposed to the class+member function design as explored in this patch (but thanks for the exploration!).
Let's discuss several use cases.

(a) if a use case just calls compress/uncompress. The class design has slightly more boilerplate as it needs to get the algorithm class, a new instance, or a singleton instance.
For each new use, the number of lines may not differ, but the involvement of a a static class member or an instance make the reader wonder whether the object will be reused or thrown away.
There is some slight cognitive burden.
The class design has a non-trivial one-shot cost to have a function returning the singleton instance.

(b) zlib compress/uncompress immediately following an availability check.

// free function
if (!compression::zlib::isAvailable())
  errs() << "cannot compress: " << compression::zlib::buildConfigurationHint();

// class
auto *algo = !compression::ZlibCompression;
if (!algo->isAvailable()) {
  errs() << "cannot compress: " << algo->buildConfigurationHint();
}

(c) zlib/zstd compress/uncompress immediately following an availability check.

// free function
if (!compression::isAvailable(format))
  errs() << "cannot compress: " << compression::buildConfigurationHint(format);

// class
std::unique_ptr<Compression> algo = make_compression(format);
if (!algo->isAvailable()) {
  errs() << "cannot compress: " << algo->buildConfigurationHint();
}

(d) compress/uncompress and an availability check are apart.

// free function
no change

// class
Store (the pointer to the) the algorithm object somewhere, or construct the pointer/object twice.

[leonardchan]

llvm/lib/Support/Compression.cpp

30–32

Perhaps for each of these, you could instead have something like:

ZStdCompressionAlgorithm *getZStdCompressionAlgorithm() {
  static ZStdCompressionAlgorithm* instance = new ZStdCompressionAlgorithm;
  return instance;
}

This way the instances are only new'd when they're actually used.

[dblaikie]

In D130516#3688123, @MaskRay wrote:

I'd like to make a few arguments for the current namespace+free function design, as opposed to the class+member function design as explored in this patch (but thanks for the exploration!).
Let's discuss several use cases.

(a) if a use case just calls compress/uncompress. The class design has slightly more boilerplate as it needs to get the algorithm class, a new instance, or a singleton instance.
For each new use, the number of lines may not differ, but the involvement of a a static class member or an instance make the reader wonder whether the object will be reused or thrown away.
There is some slight cognitive burden.
The class design has a non-trivial one-shot cost to have a function returning the singleton instance.

Though there must've been a condition that dominates this use somewhere - I'd suggest that condition could be where the algorithm is retrieved, and then passed to this code to use unconditionally.

If the algorithm object is const and raw pointers/references are used, I think it makes it clear to the reader that there's no ownership here, and it's not stateful when compressing/decompressing.

(b) zlib compress/uncompress immediately following an availability check.

// free function
if (!compression::zlib::isAvailable())
  errs() << "cannot compress: " << compression::zlib::buildConfigurationHint();

// class
auto *algo = !compression::ZlibCompression;
if (!algo->isAvailable()) {
  errs() << "cannot compress: " << algo->buildConfigurationHint();
}

I think maybe this code might end up looking like:

Algo *algo = getAlgo(Zlib)
if (!algo)
  errs() ...

It's possible that this function would return non-null even for a non-available algorithm if we wanted to communicate other things (like the cmake macro name to enable to add the functionality)

(c) zlib/zstd compress/uncompress immediately following an availability check.

// free function
if (!compression::isAvailable(format))
  errs() << "cannot compress: " << compression::buildConfigurationHint(format);

// class
std::unique_ptr<Compression> algo = make_compression(format);
if (!algo->isAvailable()) {
  errs() << "cannot compress: " << algo->buildConfigurationHint();
}

I don't think there's a need for unique_ptr here - algorithms can be constant singletons, referenced via raw const pointers/references without ownership.

& this example doesn't include the code that does the compression/decompression, which seems part of the discussion & part I find nice in that the type of compression used matches the type used in the check necessarily rather than being passed into two APIs independently.

(d) compress/uncompress and an availability check are apart.

// free function
no change

// class
Store (the pointer to the) the algorithm object somewhere, or construct the pointer/object twice.

The benefit here is that it's harder for the test to become separated from the usage - for the usage to end up becoming unconditional/incorrectly guarded.

llvm/lib/Support/Compression.cpp

30–32

Yep, I'd mentioned/suggested that (so, seconding here) elsewhere encouraging these to be singletons: https://reviews.llvm.org/D130516#3683384

And they don't even need to be 'new'd in that case, this would be fine:

ZstdCompressionAlgorithm &getZstdCompressionAlgorithm() {
  static ZstdCompressionAlgorithm C;
  return C;
}

Though I think maybe we don't need individual access to the algorithms, and it'd be fine to have only a single entry point like this:

CompressionAlgorithm *getCompressionAlgorithm(DebugCompressionType T) {
  switch (T) {
  case DebugCompressionType::ZStd: {
    static zstd::CompressionAlgorithm Zstd;
    if (zstd::isAvailable())
      return &Zstd;
  }
  ...
  }
  return nullptr;
}

(or, possibly, we want to return non-null even if it isn't available, if we include other things (like the configure macro name - so callers can use that name to print helpful error messages - but then they have to explicitly check if the algorithm is available after the call))

165–169

I don't think there's particular value in these being constexpr members - and maybe we don't need these at all just yet/could leave them out for now? It'd be great to reduce this whole patch to something more comparable with https://reviews.llvm.org/D130458

If you have plans for these other properties it might be helpful to understand what they are - they might help inform the design discussion.

(if we are keeping tnhese properties, including the string version of the name, etc - I'd think the way to do it would be for the base algorithm class to have non-static members to store these, and derived algorithm classes to pass the values into the base ctor to be stored in the members - they could even be const public members of the algorithm to be accessed directly, rather than via accessor functions (& certainly not virtual accessor functions))

[ckissane]

llvm/lib/Support/Compression.cpp

30–32

they currently already have singleton behavior i.e. llvm::compression::ZStdCompressionAlgorithm::Instance is the only place new ZStdCompressionAlgorithm() can be put into because the constructor is protected.

I'd rather not achieve "This way the instances are only new'd when they're actually used."
Because the rewards of that are relatively small, but it will make the code more verbose, I think the current pattern allows the best of both worlds of the namespace approach:
(llvm::compression::zlib::compress becomes llvm::compression::ZlibCompression->compress)
but they can be passed as class instances.

[dblaikie]

(still lots of outstanding comments from my last round, so I won't reiterate those - but waiting for some responses to them)

llvm/lib/Support/Compression.cpp

30–32

Global constructors are to be avoided in LLVM: https://llvm.org/docs/CodingStandards.html#do-not-use-static-constructors
(also these objects don't need to be dynamically allocated with new - they can be directly allocated (as static locals though, not as globals))

[ckissane]

• feat compression "enum" with methods

[MaskRay]

In D130516#3688236, @dblaikie wrote:

In D130516#3688123, @MaskRay wrote:

I'd like to make a few arguments for the current namespace+free function design, as opposed to the class+member function design as explored in this patch (but thanks for the exploration!).
Let's discuss several use cases.

(a) if a use case just calls compress/uncompress. The class design has slightly more boilerplate as it needs to get the algorithm class, a new instance, or a singleton instance.
For each new use, the number of lines may not differ, but the involvement of a a static class member or an instance make the reader wonder whether the object will be reused or thrown away.
There is some slight cognitive burden.
The class design has a non-trivial one-shot cost to have a function returning the singleton instance.

Though there must've been a condition that dominates this use somewhere - I'd suggest that condition could be where the algorithm is retrieved, and then passed to this code to use unconditionally.

If the algorithm object is const and raw pointers/references are used, I think it makes it clear to the reader that there's no ownership here, and it's not stateful when compressing/decompressing.

A pointer to a singleton compression class is isomorphic to an enum class CompressionType variable.
Using an enum variable doesn't lose any usage pattern we can do with a pointer to a singleton compression class.
An enum variable allows more patterns, as the allowed values are enumerable (we don't need to worry about -Wswitch for the uses).

Say, we do

auto *algo = !compression::ZlibCompression;
if (!algo)
  ...


algo->compress(...);

either together or apart, the result is similar to the following but with (IMO) slightly larger cognitive burden:

if (!compression::isAvailable(format))
  ...

compression::compress(format);

(b) zlib compress/uncompress immediately following an availability check.

// free function
if (!compression::zlib::isAvailable())
  errs() << "cannot compress: " << compression::zlib::buildConfigurationHint();

// class
auto *algo = !compression::ZlibCompression;
if (!algo->isAvailable()) {
  errs() << "cannot compress: " << algo->buildConfigurationHint();
}

I think maybe this code might end up looking like:

Algo *algo = getAlgo(Zlib)
if (!algo)
  errs() ...

It's possible that this function would return non-null even for a non-available algorithm if we wanted to communicate other things (like the cmake macro name to enable to add the functionality)

I think this is similarly achieved with an enum variable.
With the class based approach, a pointer has a static type of the ancestor compression class and a dynamic type of any possible algorithm.
This is not different from that: the enum variable may have a value the enum class supports.

(c) zlib/zstd compress/uncompress immediately following an availability check.

// free function
if (!compression::isAvailable(format))
  errs() << "cannot compress: " << compression::buildConfigurationHint(format);

// class
std::unique_ptr<Compression> algo = make_compression(format);
if (!algo->isAvailable()) {
  errs() << "cannot compress: " << algo->buildConfigurationHint();
}

I don't think there's a need for unique_ptr here - algorithms can be constant singletons, referenced via raw const pointers/references without ownership.

& this example doesn't include the code that does the compression/decompression, which seems part of the discussion & part I find nice in that the type of compression used matches the type used in the check necessarily rather than being passed into two APIs independently.

Thanks for clarification. Then this fits my "singleton compression classes are isomorphic to an enum CompressionType variable" argument :)

(d) compress/uncompress and an availability check are apart.

// free function
no change

// class
Store (the pointer to the) the algorithm object somewhere, or construct the pointer/object twice.

The benefit here is that it's harder for the test to become separated from the usage - for the usage to end up becoming unconditional/incorrectly guarded.

[ckissane]

• make a zlib corruption check specific

[ckissane]

• trim down compression api: remove supported()

[dblaikie]

In D130516#3694151, @MaskRay wrote:

In D130516#3688236, @dblaikie wrote:

In D130516#3688123, @MaskRay wrote:

I'd like to make a few arguments for the current namespace+free function design, as opposed to the class+member function design as explored in this patch (but thanks for the exploration!).
Let's discuss several use cases.

(a) if a use case just calls compress/uncompress. The class design has slightly more boilerplate as it needs to get the algorithm class, a new instance, or a singleton instance.
For each new use, the number of lines may not differ, but the involvement of a a static class member or an instance make the reader wonder whether the object will be reused or thrown away.
There is some slight cognitive burden.
The class design has a non-trivial one-shot cost to have a function returning the singleton instance.

Though there must've been a condition that dominates this use somewhere - I'd suggest that condition could be where the algorithm is retrieved, and then passed to this code to use unconditionally.

If the algorithm object is const and raw pointers/references are used, I think it makes it clear to the reader that there's no ownership here, and it's not stateful when compressing/decompressing.

A pointer to a singleton compression class is isomorphic to an enum class CompressionType variable.

I don't mean to suggest that either design is fundamentally more or less functional - I'm totally OK with/agree that both design directions allow the implementation of all the desired final/end-user-visible functionality.

I'm trying to make a point about which, I think, achieves that goal in a "better" way - that's the space of design discussions, I think - what kinds of (developer, maintenance, etc) costs different designs incur.

Using an enum variable doesn't lose any usage pattern we can do with a pointer to a singleton compression class.

I agree that either design doesn't change what's possible - I do, though, think that the "usage patterns" are meaningfully different between the two designs.

An enum variable allows more patterns, as the allowed values are enumerable (we don't need to worry about -Wswitch for the uses).

Say, we do

auto *algo = !compression::ZlibCompression;
if (!algo)
  ...


algo->compress(...);

either together or apart, the result is similar to the following but with (IMO) slightly larger cognitive burden:

if (!compression::isAvailable(format))
  ...

compression::compress(format);

Specifically two APIs that are related (it's important/necessary to check for availability before calling compress or decompress) in their contracts but unrelated in their API use makes it easier to misuse the APIs and have a situation where the availability check doesn't cover the usage. That's what I think is important/important to discuss here.

(b) zlib compress/uncompress immediately following an availability check.

// free function
if (!compression::zlib::isAvailable())
  errs() << "cannot compress: " << compression::zlib::buildConfigurationHint();

// class
auto *algo = !compression::ZlibCompression;
if (!algo->isAvailable()) {
  errs() << "cannot compress: " << algo->buildConfigurationHint();
}

I think maybe this code might end up looking like:

Algo *algo = getAlgo(Zlib)
if (!algo)
  errs() ...

It's possible that this function would return non-null even for a non-available algorithm if we wanted to communicate other things (like the cmake macro name to enable to add the functionality)

I think this is similarly achieved with an enum variable.
With the class based approach, a pointer has a static type of the ancestor compression class and a dynamic type of any possible algorithm.
This is not different from that: the enum variable may have a value the enum class supports.

I agree that the code is similar in either case, but with a small difference that is important to me - that accessing the algorithm necessarily (to some degree - you could still have code that doesn't test the condition/dereferences null, the same way that code can dereference an empty Optional without checking first - but at least the API I'm suggesting makes clear there's a connection between availability and usage).

(c) zlib/zstd compress/uncompress immediately following an availability check.

// free function
if (!compression::isAvailable(format))
  errs() << "cannot compress: " << compression::buildConfigurationHint(format);

// class
std::unique_ptr<Compression> algo = make_compression(format);
if (!algo->isAvailable()) {
  errs() << "cannot compress: " << algo->buildConfigurationHint();
}

I don't think there's a need for unique_ptr here - algorithms can be constant singletons, referenced via raw const pointers/references without ownership.

& this example doesn't include the code that does the compression/decompression, which seems part of the discussion & part I find nice in that the type of compression used matches the type used in the check necessarily rather than being passed into two APIs independently.

Thanks for clarification. Then this fits my "singleton compression classes are isomorphic to an enum CompressionType variable" argument :)

I don't understand what you're saying here. Could you rephrase/expand a bit?

[ckissane]

• CompressionKind: clean up param names to == op

[ckissane]

@dblaikie, @MaskRay I think I have worked out something that is the best of both worlds:
none compression is represented simply as a none type for use cases that will use Optional<CompressionKind>.
once you have a CompressionKind itself you can pass it around as a value (because a CompressionKind is just a struct containing one uint8_t (a fake "enum")).
due to my operator overloading, you can do stuff like this:

llvm::compression::OptionalCompressionKind OptionalCompressionScheme =
          llvm::compression::getOptionalCompressionKind(CompressionSchemeId);
      if (!OptionalCompressionScheme) {
        return llvm::MemoryBuffer::getMemBuffer(Blob, Name, true);
      }
      llvm::compression::CompressionKind CompressionScheme =
          *OptionalCompressionScheme;
      if (!CompressionScheme) {
        Error("compression class " +
              (CompressionScheme->getName() + " is not available").str());
        return nullptr;
      }
      SmallVector<uint8_t, 0> Uncompressed;
      if (llvm::Error E = CompressionScheme->decompress(
              llvm::arrayRefFromStringRef(Blob), Uncompressed, Record[0])) {
        Error("could not decompress embedded file contents: " +
              llvm::toString(std::move(E)));
        return nullptr;
      }
      return llvm::MemoryBuffer::getMemBufferCopy(
          llvm::toStringRef(Uncompressed), Name);

(excerpt from ASTReader.cpp)

you can even do calls like

llvm::compression::CompressionKind::Zlib->decompress(...)
(this can be useful in cases like elf decompression where you might switch on debug section type and in a switch case for DebugCompressionType::Z)

[dblaikie]

The current code here still seems more complicated than I'd prefer - looks like currently the size/speed/default levels are currently unused, so maybe we can omit those for now, knowing they will be added?
And the CompressionKind with all its operator overloads seems like a lot of surface area that is pretty non-obvious for usage - boolean testable, logical operator overloads, etc.
Could we have only one decompress/compress function each, for now?
& maybe leave out the name/enum from the base class for now, add it in later (& I think I mentionted in another comment those properties can be non-virtual, maybe even direct const members - passed into the base through the ctor from the derived class)

Maybe it's easier if I either post a patch, or at least more explicitly flesh out what I'm picturing/proposing/suggesting:
Header:

struct CompressionAlgorithm {
  virtual void Compress(...);
  virtual void Decompress(...);
};
enum class CompressionType {
  Zlib, Zstd
};
CompressionAlgorithm *getCompressionAlgorithm(CompressionType);

Implementation:

#if LLVM_ENABLE_ZLIB
struct ZlibCompressionAlgorthim : CompressionAlgorithm {
  void Compress(...) { ... }
  void Decompress(...) { ...}
}
#endif
...
CompressionAlgorithm *getCompressionAlgorithm(CompressionType T) {
  switch (T) {
  case CompressionType::Zlib: {
#if LLVM_ENABLE_ZLIB
    static ZlibCompressionAlgorithm A;
    return &A;
#else
    break;
#endif
  }
...
  }
  return nullptr;
}

Usage:

if (CompressionAlgorithm *C = getCompressionAlgorithm(CompressionType::Zlib) {
  C->compress(...);
}

And, yeah, I think it'd be suitable to eventually add name, type, size/speed/default levels:

struct CompressionAlgorithm {
  const StringRef Name;
  const CompressionType Type;
  const int DefaultLevel;
  const int BestSizeLevel;
  const int BestSpeedLevel;
  virtual void Compress(...);
  virtual void Decompress(...);
protected:
  CompressionAlgorithm(StringRef Name, CompressionType Type, ...) : Name(Name), Type(Type), ... {}
}

struct ZlibCompressionAlgorithm : CompressionAlgorithm {
  ZlibCompressionAlgorithm() : CompressionAlgorithm("zlib", CompressionType::Zlib, 5, 10, 1) { }
  /* as before */
};
...

Though those can be added as needed - good to keep in mind that they're a useful direction to go, but might simplify the review/discussion to omit them for now.

[dblaikie]

I think I have worked out something that is the best of both worlds:

I think @MaskRay's main concern, which I share to a degree, is that there's a lot of code/complexity here that doesn't currently seem warranted by the size of the problem. So adding more implementation complexity to this patch, even if it does provide some of the benefits (though I don't think the ability to do boolean logic, etc, is the main concern either myself or @MaskRay have - either way we'll have the enum) it's adding a lot more implementation complexity, which is something we're trying to address/reduce.

[MaskRay]

In D130516#3694366, @dblaikie wrote:

In D130516#3694151, @MaskRay wrote:

In D130516#3688236, @dblaikie wrote:

In D130516#3688123, @MaskRay wrote:

I'd like to make a few arguments for the current namespace+free function design, as opposed to the class+member function design as explored in this patch (but thanks for the exploration!).
Let's discuss several use cases.

(a) if a use case just calls compress/uncompress. The class design has slightly more boilerplate as it needs to get the algorithm class, a new instance, or a singleton instance.
For each new use, the number of lines may not differ, but the involvement of a a static class member or an instance make the reader wonder whether the object will be reused or thrown away.
There is some slight cognitive burden.
The class design has a non-trivial one-shot cost to have a function returning the singleton instance.

Though there must've been a condition that dominates this use somewhere - I'd suggest that condition could be where the algorithm is retrieved, and then passed to this code to use unconditionally.

If the algorithm object is const and raw pointers/references are used, I think it makes it clear to the reader that there's no ownership here, and it's not stateful when compressing/decompressing.

A pointer to a singleton compression class is isomorphic to an enum class CompressionType variable.

I don't mean to suggest that either design is fundamentally more or less functional - I'm totally OK with/agree that both design directions allow the implementation of all the desired final/end-user-visible functionality.

I'm trying to make a point about which, I think, achieves that goal in a "better" way - that's the space of design discussions, I think - what kinds of (developer, maintenance, etc) costs different designs incur.

[...]

I don't understand what you're saying here. Could you rephrase/expand a bit?

Maybe it's easier if I either post a patch, or at least more explicitly flesh out what I'm picturing/proposing/suggesting:

I agree that it is easier if you post a patch so that we can have discussion on concrete code. I think we are now at the "a picture is worth a thousand words" stage:)

[ckissane]

In D130516#3694422, @dblaikie wrote:

The current code here still seems more complicated than I'd prefer - looks like currently the size/speed/default levels are currently unused, so maybe we can omit those for now, knowing they will be added?
And the CompressionKind with all its operator overloads seems like a lot of surface area that is pretty non-obvious for usage - boolean testable, logical operator overloads, etc.
Could we have only one decompress/compress function each, for now?
& maybe leave out the name/enum from the base class for now, add it in later (& I think I mentionted in another comment those properties can be non-virtual, maybe even direct const members - passed into the base through the ctor from the derived class)

Yes, I can continue to trim down the implementation! I agree with your sentiment.

Maybe it's easier if I either post a patch, or at least more explicitly flesh out what I'm picturing/proposing/suggesting:
Header:

struct CompressionAlgorithm {
  virtual void Compress(...);
  virtual void Decompress(...);
};
enum class CompressionType {
  Zlib, Zstd
};
CompressionAlgorithm *getCompressionAlgorithm(CompressionType);

Implementation:

#if LLVM_ENABLE_ZLIB
struct ZlibCompressionAlgorthim : CompressionAlgorithm {
  void Compress(...) { ... }
  void Decompress(...) { ...}
}
#endif
...
CompressionAlgorithm *getCompressionAlgorithm(CompressionType T) {
  switch (T) {
  case CompressionType::Zlib: {
#if LLVM_ENABLE_ZLIB
    static ZlibCompressionAlgorithm A;
    return &A;
#else
    break;
#endif
  }
...
  }
  return nullptr;
}

I agree with some of this, I have some strong thoughts I would like to work out about the whole nullptr being none or unsupported a little preemptively IMO.

Usage:

if (CompressionAlgorithm *C = getCompressionAlgorithm(CompressionType::Zlib) {

C->compress(...);

}

currently, you can do

if (CompressionKind C = CompressionKind::Zlib) {
  C->compress(...);
}

And, yeah, I think it'd be suitable to eventually add name, type, size/speed/default levels:

struct CompressionAlgorithm {
  const StringRef Name;
  const CompressionType Type;
  const int DefaultLevel;
  const int BestSizeLevel;
  const int BestSpeedLevel;
  virtual void Compress(...);
  virtual void Decompress(...);
protected:
  CompressionAlgorithm(StringRef Name, CompressionType Type, ...) : Name(Name), Type(Type), ... {}
}

struct ZlibCompressionAlgorithm : CompressionAlgorithm {
  ZlibCompressionAlgorithm() : CompressionAlgorithm("zlib", CompressionType::Zlib, 5, 10, 1) { }
  /* as before */
};
...

Though those can be added as needed - good to keep in mind that they're a useful direction to go, but might simplify the review/discussion to omit them for now.

[ckissane]

compression api: greatly simplify implementation
+ cuts around 100 lines of code from compression.h and compression.cpp

[dblaikie]

Yes, I can continue to trim down the implementation! I agree with your sentiment.

Thanks! This update helps - though I think we'll still want to further isolate the different pieces of this change/reduce this further.

I agree with some of this, I have some strong thoughts I would like to work out about the whole nullptr being none or unsupported a little preemptively IMO.

Could you clarify what use cases you have in mind that require the nuance between none and unsupported? (arguably this accessor function could assert when passed None - would that make it simpler? Then the only null return would be unsupported)

Usage:

if (CompressionAlgorithm *C = getCompressionAlgorithm(CompressionType::Zlib) {

C->compress(...);

}

currently, you can do

if (CompressionKind C = CompressionKind::Zlib) {
  C->compress(...);
}

The implementation complexity is a concern too, though. I think having CompressionKind, boolean conversions and logical operator overloads, etc, in addition to the CompressionAlgorithm doesn't seem to provide enough to justify the complexity - but perhaps I'm missing some context/understanding of the values those features provide?

What sort of use cases do you have in mind that necessitate that complexity/functionality? (specifically I see a lot of || llvm::NoneType() which seems really obtuse/unclear why a user of the API would think to do that/understand that was the right/necessary thing to do)

Maybe for comparison purposes it'd be good not to replace this API but to add it on top of the underlying API so only one callsite can be updated, rather than all the changes necessary to update all clients in one go (& in this way maybe omit some of the functionality in this first patch, since it won't have to cover all use cases - eg: those extra fields (name/compression levels (best size/speed/default), etc) could possibly be omitted from the first version of this patch, so the patch only adds enough functionality for one of the compresion clients (like MC, to match/compare with @MaskRay's patch) rather than all of them)

llvm/include/llvm/Support/Compression.h

47–50

Could we skip this wrapper & just have the underlying function (& also we shouldn't be overloading by case like this anyway - please name all the decompress/compress functions with the same case/spelling)

[ckissane]

llvm/include/llvm/Support/Compression.h

47–50

good point

[ckissane]

• remove uppercase Compress, Decompress

[ckissane]

• remove compression kind || && overload

[ckissane]

@dblaikie @MaskRay I would like it if you could all take another look.

In response to @dblaikie 's comments about implementation weight I have greatly simplified the implementation, including removing extra capitalized function overloads (Compress, Decompress), and removing || and && operator overrides. Also adopting the recently suggested class impl in part.

I also believe similar semantics to the nullptr suggestion have been achieved, due to the bool cast returning supported status, and the Unknown type acting as a nullptr of sorts, being always unsupported and -1 (255) as a uint8. Optional(Unknown) is returned from getOptionalCompressionKind(uint8_t) when it is not 0 (NoneType()), 1 (Optional(Zlib)), or 2 (Optional(ZStd)).

In places where explict compression must be used CompressionKind can be passed around, and possibly optional (sometimes none) compression is represented as llvm::Optional<CompressionKind> which I have type aliased as OptionalCompressionKind

[ckissane]

• fix some nits in Compression.h

[ckissane]

• move if into switch

[leonardchan]

clang-tools-extra/clangd/index/Serialization.cpp

250–251

nit: I think error accepts format-like arguments, so you could have something similar to above with:

return error("Compressed string table, but {0} is unavailable", CompressionScheme->Name);

clang/lib/Serialization/ASTReader.cpp

1468–1470

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

1474–1475

I think this Error takes a StringRef, so I think you could have:

return Error("compression class " + CompressionScheme->Name + " is not available");

lld/ELF/InputSection.cpp

1229

typename might not be needed here

llvm/include/llvm/Support/Compression.h

27–165

https://llvm.org/docs/CodingStandards.html#use-of-class-and-struct-keywords since the ctor is non-public

39

this-> might not be needed here

85

I think this cast might not be needed

105

I think this cast might not be needed

llvm/lib/ObjCopy/ELF/ELFObject.cpp

442–461

What's the explanation for having the llvm_unreachable branch and getting the compression type? I would've thought this section would just be:

if (Error Err1 = compression::CompressionKind::Zlib->decompress(
        Compressed, DecompressedContent, static_cast<size_t>(Sec.Size))) {
  return createStringError(errc::invalid_argument,
                           "'" + Sec.Name +
                               "': " + toString(std::move(Err1)));
}

which looks like it would have identical behavior to the old code.

530–538

Same here. Should this just be compression::CompressionKind::Zlib->compress(OriginalData, CompressedData);? If this is in preparation for the ELF+zstd changes, perhaps we should save those for another patch once that lands?

llvm/lib/Support/Compression.cpp

129–130

nit: add overrides to be more explicit these are virtual methods

143–144

If the llvm_unreachables should be the default implementation for all subclasses, perhaps the [de]compress methods should be regular virtual with these default implementations rather than abstract virtual.

209–212

Same here

226

I think this cast might not be needed

236–239

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

or perhaps just

return left ? left : NoneType();

[dblaikie]

llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp

57–63

This still seems like a lot of hoops to jump through - why "noneIfUnsupported" rather than either having the compression scheme (I think it could be the CompressionAlgorithm itself, rather than having the separate OptionalCompressionKind abstraction) either be null itself, or expose an "isAvailable" operation directly on the CompressionAlgorithm?

Even if the CompressionKind/OptionalCompressionKind/CompressionAlgorithm abstractions are kept, I'm not sure why the above code is preferred over, say:

if (Compress && DoInstrProfNameCompression && OptionalCompressionScheme /* .isAvailable(), if we want to be more explicit */) {
  ...
}

What's the benefit that noneIfUnsupported is providing? (& generally I'd expect the Compress && DoInstrProfNameCompression to be tested/exit early before even naming/constructing/querying/doing anything with the compression scheme/algorithm/etc - so there'd be no need to combine the tests for availability and the tests for whether compression was requested)

Perhaps this API is motivated by a desire to implement something much closer to the original code than is necessary/suitable? Or some other use case/benefit I'm not quite understanding yet?

[ckissane]

llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp

57–63

I shall remove noneIfUnsupported. You express good points, we can simply check if(OptionalCompressionScheme && *OptionalCompressionScheme) where necessary.

[ckissane]

llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp

57–63

though it will make a lot of existing code patterns less clear, and more verbose

[ckissane]

llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp

57–63

and sometimes you really do need to re code the exact thing noneIfUnsupported encapsulates...

[dblaikie]

llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp

57–63

Are there examples within LLVM that you can show compare/contrast noneIfUnsupported helps?

[ckissane]

• cleanup some compression nits

[ckissane]

llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp

57–63

yes, I'll paste a couple here

[ckissane]

llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp

57–63

Ok, So I believe I was mistaken.
In older versions of this patch there was a none compression implementation that just did a memcpy, this made a natural fall through state, which made this type of pattern advantageous.
However, this is no longer the case.
Hence I will remove this without further adue.
Thank you for your astute observation!

[ckissane]

• remove OptionalCompressionKind noneIfUnsupported

[ckissane]

clang/lib/Serialization/ASTReader.cpp

1474–1475

unfortunately that doesn't work (I tried)

[ckissane]

• format error string

[ckissane]

• fix InputSection decompress issue

[ckissane]

• Merge remote-tracking branch 'origin/main' into ckissane.compression-class-simple

[dblaikie]

This is looking pretty close to what I've been picturing - the only thing remaining is that I think we could get rid of CompressionKind and access the CompressionAlgorithm directly - basically the contents of CompressionKind::operator-> could be a free/public function const CompressionAlgorithm &getCompressionAlgorithm(enum class CompressionKind); - and have it return a reference to the local static implementation, with a none implementation (for those profile cases where you want to pass in an algorithm if it's available, or none) and one implementation for each of zlib/zstd?

clang-tools-extra/clangd/index/Serialization.cpp

237

What purpose is this expression serving? (isn't it a tautology, given that CompressionScheme was initialized a couple of lines back with CompressionKind::Zlib?

clang/lib/Serialization/ASTWriter.cpp

2003–2012

Generally LLVM style rolls these together

llvm/lib/Object/Decompressor.cpp

41–50

Maybe leave this code more like it was before - it can turn into a switch over ELFCompressionSchemeId when Zstd is added here.

llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp

122–132

Be nice to share the same CompressionKind

llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp

54–55

Why/how does OptionalCompressionKind end up in here, compared with this (suggested edit)?

llvm/lib/ProfileData/InstrProf.cpp

463

Yeah, this seems awkward, but I see what you're getting at - if you're going to pass around an algorithm to use, and this particular kind of use case wants to collapse the "algorithm not available" and "no compression was requested" - so, yeah, for this sort of use case I can see the valid in having a null compression algorithm implementation. That shouldn't add a lot of complexity to the implementation and simplify the usage so it doesn't have two layers of "not present" state.

495

Presumably this doesn't need to test OptionalCompressionScheme here, though, since it's tested in the implementation?

llvm/lib/ProfileData/SampleProfReader.cpp

880–885

Nice to pull out a common variable rather than accessing CompressionKind::Zlib twice independently (otherwise we probably might as well go with the more direct API @MaskRay has proposed)

[ckissane]

• address review comments

[ckissane]

In D130516#3703691, @dblaikie wrote:

This is looking pretty close to what I've been picturing - the only thing remaining is that I think we could get rid of CompressionKind and access the CompressionAlgorithm directly - basically the contents of CompressionKind::operator-> could be a free/public function const CompressionAlgorithm &getCompressionAlgorithm(enum class CompressionKind); - and have it return a reference to the local static implementation, with a none implementation (for those profile cases where you want to pass in an algorithm if it's available, or none) and one implementation for each of zlib/zstd?

I can see what you are asking for, however since its behavior is essentially the same, and still uses both enum values and class implementations, I don't see any practical advantages (though if there is something I am failing to observe let me know).
Additionally, I can see some disadvantages in largely increasing code verbosity across the codebase.
A snippet of an example from elf: if(CompressionKind::Zlib) CompressionKind::Zlib->compress...
would turn into if(getCompressionAlgorithm(CompressionKind::Zlib)) getCompressionAlgorithm(CompressionKind::Zlib)->compress...
(assuming bool operator overload is also moved to class)
Because of this I am leaning away from such an implementation change.

[MaskRay]

I have only taken very brief look at the new version. Having an enum class CompressionKind with a parallel CompressionAlgorithm seems redundant.
friend CompressionAlgorithm *CompressionKind::operator->() const; looks magical.

I hope that someone insisting on object-oriented design can put up a version with less boilerplate to compete with D130506.

[dblaikie]

In D130516#3710903, @ckissane wrote:

In D130516#3703691, @dblaikie wrote:

This is looking pretty close to what I've been picturing - the only thing remaining is that I think we could get rid of CompressionKind and access the CompressionAlgorithm directly - basically the contents of CompressionKind::operator-> could be a free/public function const CompressionAlgorithm &getCompressionAlgorithm(enum class CompressionKind); - and have it return a reference to the local static implementation, with a none implementation (for those profile cases where you want to pass in an algorithm if it's available, or none) and one implementation for each of zlib/zstd?

I can see what you are asking for, however since its behavior is essentially the same, and still uses both enum values and class implementations, I don't see any practical advantages (though if there is something I am failing to observe let me know).

The intent is to simplify both implementation and usage because this currently feels like overkill for a fairly small abstraction benefit (compared to @MaskRay's posted alternative, for instance) - we're abstracting only a handful of use cases over only 2 implementations, so this shouldn't be too complicated/overengineered.

Additionally, I can see some disadvantages in largely increasing code verbosity across the codebase.
A snippet of an example from elf: if(CompressionKind::Zlib) CompressionKind::Zlib->compress...
would turn into if(getCompressionAlgorithm(CompressionKind::Zlib)) getCompressionAlgorithm(CompressionKind::Zlib)->compress...

I think in either case this should be changed to something like this (specifically avoid writing CompressionKind::Zlib twice):

if (auto C = CompressionKind::Zlib)
  C->compress...

(assuming bool operator overload is also moved to class)
Because of this I am leaning away from such an implementation change.

[dblaikie]

In D130516#3710972, @MaskRay wrote:

I have only taken very brief look at the new version. Having an enum class CompressionKind with a parallel CompressionAlgorithm seems redundant.
friend CompressionAlgorithm *CompressionKind::operator->() const; looks magical.

I hope that someone insisting on object-oriented design can put up a version with less boilerplate to compete with D130506.

Posted something more comparable to D130506 in D131638 - hard to compare, though - D130506 is additive, whereas D131638 and this D130516 are more replacements - though a different with my change there is at least for the initial patch leaving the old APIs in place, with the intent to incrementally change the usages until the old API can be removed.

clang-tools-extra/clangd/index/Serialization.cpp

 #include "llvm/Support/raw_ostream.h"
 #include <cstdint>
 #include <vector>
 
 namespace clang {
 namespace clangd {
 namespace {
 
 // IO PRIMITIVES
 // We use little-endian 32 bit ints, sometimes with variable-length encoding.
 //
 // Variable-length int encoding (varint) uses the bottom 7 bits of each byte
 // to encode the number, and the top bit to indicate whether more bytes follow.
 // e.g. 9a 2f means [0x1a and keep reading, 0x2f and stop].
 // This represents 0x1a | 0x2f<<7 = 6042.
 // A 32-bit integer takes 1-5 bytes to encode; small numbers are more compact.
 
void finalize(llvm::raw_ostream &OS) {
     for (unsigned I = 0; I < Sorted.size(); ++I)
       Index.try_emplace({Sorted[I].data(), Sorted[I].size()}, I);
 
     std::string RawTable;
     for (llvm::StringRef S : Sorted) {
       RawTable.append(std::string(S));
       RawTable.push_back(0);
     }
-    if (llvm::compression::zlib::isAvailable()) {
+    llvm::compression::CompressionAlgorithm CompressionScheme =
+        llvm::compression::ZlibCompressionAlgorithm();
+    if (CompressionScheme.supported()) {
       llvm::SmallVector<uint8_t, 0> Compressed;
-      llvm::compression::zlib::compress(llvm::arrayRefFromStringRef(RawTable),
-                                        Compressed);
+      CompressionScheme.compress(llvm::arrayRefFromStringRef(RawTable),
       write32(RawTable.size(), OS);
       OS << llvm::toStringRef(Compressed);
     } else {
       write32(0, OS); // No compression.
       OS << RawTable;
     }
   }
   // Get the ID of an string, which must be interned. Table must be finalized.
llvm::Expected<StringTableIn> readStringTable(llvm::StringRef Data) {
   size_t UncompressedSize = R.consume32();
   if (R.err())
     return error("Truncated string table");
 
   llvm::StringRef Uncompressed;
   llvm::SmallVector<uint8_t, 0> UncompressedStorage;
   if (UncompressedSize == 0) // No compression
     Uncompressed = R.rest();
-  else if (llvm::compression::zlib::isAvailable()) {
+  else {
+    llvm::compression::CompressionAlgorithm CompressionScheme =
+        llvm::compression::ZlibCompressionAlgorithm();
+    if (CompressionScheme.supported()) {
       // Don't allocate a massive buffer if UncompressedSize was corrupted
       // This is effective for sharded index, but not big monolithic ones, as
       // once compressed size reaches 4MB nothing can be ruled out.
       // Theoretical max ratio from https://zlib.net/zlib_tech.html
       constexpr int MaxCompressionRatio = 1032;
       if (UncompressedSize / MaxCompressionRatio > R.rest().size())
-      return error("Bad stri table: uncompress {0} -> {1} bytes is implausible",
-                   R.rest().size(), UncompressedSize);
+        return error(
+            "Bad stri table: uncompress {0} -> {1} bytes is implausible",
 
-    if (llvm::Error E = llvm::compression::zlib::uncompress(
+      if (llvm::Error E = CompressionScheme.decompress(
               llvm::arrayRefFromStringRef(R.rest()), UncompressedStorage,
               UncompressedSize))
         return std::move(E);
       Uncompressed = toStringRef(UncompressedStorage);
     } else
-    return error("Compressed string table, but zlib is unavailable");
+      return error("Compressed string table, but " +
+                   (CompressionScheme.name + " is unavailable").str());
+  }
 
   StringTableIn Table;
   llvm::StringSaver Saver(Table.Arena);
   R = Reader(Uncompressed);
   for (Reader R(Uncompressed); !R.eof();) {
     auto Len = R.rest().find(0);
     if (Len == llvm::StringRef::npos)
       return error("Bad string table: not null terminated");

clang-tools-extra/clangd/unittests/SerializationTests.cpp

TEST(SerializationTest, NoCrashOnBadArraySize) {
   ASSERT_TRUE(!CorruptParsed);
   EXPECT_EQ(llvm::toString(CorruptParsed.takeError()),
             "malformed or truncated include uri");
 }
 
 // Check we detect invalid string table size size without allocating it first.
 // If this detection fails, the test should allocate a huge array and crash.
 TEST(SerializationTest, NoCrashOnBadStringTableSize) {
-  if (!llvm::compression::zlib::isAvailable()) {
+  if (!llvm::compression::ZlibCompressionAlgorithm().supported()) {
     log("skipping test, no zlib");
     return;
   }
 
   // First, create a valid serialized file.
   auto In = readIndexFile(YAML);
   ASSERT_FALSE(!In) << In.takeError();
   IndexFileOut Out(*In);

clang/lib/Driver/ToolChains/Clang.cpp

static void RenderDebugInfoCompressionArgs(const ArgList &Args,
   const Arg *A = Args.getLastArg(options::OPT_gz_EQ);
   if (!A)
     return;
   if (checkDebugInfoOption(A, Args, D, TC)) {
     StringRef Value = A->getValue();
     if (Value == "none") {
       CmdArgs.push_back("--compress-debug-sections=none");
     } else if (Value == "zlib") {
-      if (llvm::compression::zlib::isAvailable()) {
+      if (llvm::compression::ZlibCompressionAlgorithm().supported()) {
         CmdArgs.push_back(
             Args.MakeArgString("--compress-debug-sections=" + Twine(Value)));
       } else {
         D.Diag(diag::warn_debug_compression_unavailable);
       }
     } else {
       D.Diag(diag::err_drv_unsupported_option_argument)
           << A->getOption().getName() << Value;

clang/lib/Serialization/ASTReader.cpp

Expected<unsigned> MaybeRecCode =
         SLocEntryCursor.readRecord(Code, Record, &Blob);
     if (!MaybeRecCode) {
       Error(MaybeRecCode.takeError());
       return nullptr;
     }
     unsigned RecCode = MaybeRecCode.get();
 
     if (RecCode == SM_SLOC_BUFFER_BLOB_COMPRESSED) {
-      if (!llvm::compression::zlib::isAvailable()) {
-        Error("zlib is not available");
+      uint8_t CompressionSchemeId = static_cast<uint8_t>(
+          llvm::compression::ZlibCompressionAlgorithm().AlgorithmId);
+      llvm::compression::CompressionAlgorithm CompressionScheme =
+          llvm::compression::CompressionAlgorithmFromId(CompressionSchemeId);
+      if (!CompressionScheme.supported()) {
+        Error("compression class " +
+              (CompressionScheme.name + " is not available").str());
         return nullptr;
       }
       SmallVector<uint8_t, 0> Uncompressed;
-      if (llvm::Error E = llvm::compression::zlib::uncompress(
+      if (llvm::Error E = CompressionScheme.decompress(
               llvm::arrayRefFromStringRef(Blob), Uncompressed, Record[0])) {
         Error("could not decompress embedded file contents: " +
               llvm::toString(std::move(E)));
         return nullptr;
       }
       return llvm::MemoryBuffer::getMemBufferCopy(
           llvm::toStringRef(Uncompressed), Name);
     } else if (RecCode == SM_SLOC_BUFFER_BLOB) {

clang/lib/Serialization/ASTWriter.cpp

 
 static void emitBlob(llvm::BitstreamWriter &Stream, StringRef Blob,
                      unsigned SLocBufferBlobCompressedAbbrv,
                      unsigned SLocBufferBlobAbbrv) {
   using RecordDataType = ASTWriter::RecordData::value_type;
 
   // Compress the buffer if possible. We expect that almost all PCM
   // consumers will not want its contents.
+  llvm::compression::CompressionAlgorithm CompressionScheme =
+      llvm::compression::ZlibCompressionAlgorithm();
+
+  if (CompressionScheme.supported()) {
+
     SmallVector<uint8_t, 0> CompressedBuffer;
-  if (llvm::compression::zlib::isAvailable()) {
-    llvm::compression::zlib::compress(
-        llvm::arrayRefFromStringRef(Blob.drop_back(1)), CompressedBuffer);
+
+    CompressionScheme.compress(llvm::arrayRefFromStringRef(Blob.drop_back(1)),
+                               CompressedBuffer);
     RecordDataType Record[] = {SM_SLOC_BUFFER_BLOB_COMPRESSED, Blob.size() - 1};
     Stream.EmitRecordWithBlob(SLocBufferBlobCompressedAbbrv, Record,
                               llvm::toStringRef(CompressedBuffer));
     return;
   }
 
   RecordDataType Record[] = {SM_SLOC_BUFFER_BLOB};
   Stream.EmitRecordWithBlob(SLocBufferBlobAbbrv, Record, Blob);
 }

lld/ELF/Driver.cpp

for (uint32_t i = 0, size = cgProfile.size(); i < size; ++i) {
       if (from && to)
         config->callGraphProfile[{from, to}] += cgpe.cgp_weight;
     }
   }
 }
 
 static bool getCompressDebugSections(opt::InputArgList &args) {
   StringRef s = args.getLastArgValue(OPT_compress_debug_sections, "none");
-  if (s == "none")
+  if (s == "none") {
     return false;
-  if (s != "zlib")
-    error("unknown --compress-debug-sections value: " + s);
-  if (!compression::zlib::isAvailable())
+  } else if (s == "zlib") {
+    if (!compression::ZlibCompressionAlgorithm().supported())
       error("--compress-debug-sections: zlib is not available");
+  } else {
+    error("unknown --compress-debug-sections value: " + s);
+  }
+
   return true;
 }
 
 static StringRef getAliasSpelling(opt::Arg *arg) {
   if (const opt::Arg *alias = arg->getAlias())
     return alias->getSpelling();
   return arg->getSpelling();
 }

lld/ELF/InputSection.cpp

void InputSectionBase::uncompress() const {
   size_t size = uncompressedSize;
   uint8_t *uncompressedBuf;
   {
     static std::mutex mu;
     std::lock_guard<std::mutex> lock(mu);
     uncompressedBuf = bAlloc().Allocate<uint8_t>(size);
   }
 
-  if (Error e = compression::zlib::uncompress(rawData, uncompressedBuf, size))
+  if (Error e = compression::ZlibCompressionAlgorithm().decompress(
+          rawData, uncompressedBuf, size))
     fatal(toString(this) +
           ": uncompress failed: " + llvm::toString(std::move(e)));
   rawData = makeArrayRef(uncompressedBuf, size);
   uncompressedSize = -1;
 }
 
 template <class ELFT> RelsOrRelas<ELFT> InputSectionBase::relsOrRelas() const {
   if (relSecIdx == 0)
template <typename ELFT> void InputSectionBase::parseCompressedHeader() {
   // New-style header
   if (rawData.size() < sizeof(typename ELFT::Chdr)) {
     error(toString(this) + ": corrupted compressed section");
     return;
   }
 
   auto *hdr = reinterpret_cast<const typename ELFT::Chdr *>(rawData.data());
   if (hdr->ch_type == ELFCOMPRESS_ZLIB) {
-    if (!compression::zlib::isAvailable())
+    if (!compression::ZlibCompressionAlgorithm().supported())
       error(toString(this) + " is compressed with ELFCOMPRESS_ZLIB, but lld is "
                              "not built with zlib support");
   } else {
     error(toString(this) + ": unsupported compression type (" +
           Twine(hdr->ch_type) + ")");
     return;
   }
 
   uncompressedSize = hdr->ch_size;
   alignment = std::max<uint32_t>(hdr->ch_addralign, 1);
-  rawData = rawData.slice(sizeof(*hdr));
 }
 
 InputSection *InputSectionBase::getLinkOrderDep() const {
   assert(flags & SHF_LINK_ORDER);
   if (!link)
     return nullptr;
   return cast<InputSection>(file->getSections()[link]);
 }
template <class ELFT> void InputSection::writeTo(uint8_t *buf) {
   if (LLVM_UNLIKELY(type == SHT_GROUP)) {
     copyShtGroup<ELFT>(buf);
     return;
   }
 
   // If this is a compressed section, uncompress section contents directly
   // to the buffer.
   if (uncompressedSize >= 0) {
+    auto *hdr = reinterpret_cast<const typename ELFT::Chdr *>(rawData.data());
     size_t size = uncompressedSize;
-    if (Error e = compression::zlib::uncompress(rawData, buf, size))
+    if (hdr->ch_type == ELFCOMPRESS_ZLIB) {
+      if (!compression::ZlibCompressionAlgorithm().supported()) {
+        error(toString(this) +
+              " is compressed with ELFCOMPRESS_ZLIB, but lld is "
+              "not built with zlib support");
+      } else {
+        if (Error e = compression::ZlibCompressionAlgorithm().decompress(
+                rawData.slice(sizeof(typename ELFT::Chdr)), buf, size))
           fatal(toString(this) +
                 ": uncompress failed: " + llvm::toString(std::move(e)));
+      }
+    } else {
+      error(toString(this) + ": unsupported compression type (" +
+            Twine(hdr->ch_type) + ")");
+      return;
+    }
     uint8_t *bufEnd = buf + size;
     relocate<ELFT>(buf, bufEnd);
     return;
   }
 
   // Copy section contents from source object file to output file
   // and then apply relocations.
   memcpy(buf, rawData.data(), rawData.size());

llvm/include/llvm/Object/Decompressor.h

 //===-- Decompressor.h ------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===/
 
 #ifndef LLVM_OBJECT_DECOMPRESSOR_H
 #define LLVM_OBJECT_DECOMPRESSOR_H
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Compression.h"
 #include "llvm/Support/Error.h"
 
 namespace llvm {
 namespace object {
 
 /// Decompressor helps to handle decompression of compressed sections.
 class Decompressor {
 public:
public:
   Error decompress(MutableArrayRef<uint8_t> Buffer);
 
   /// Return memory buffer size required for decompression.
   uint64_t getDecompressedSize() { return DecompressedSize; }
 
 private:
   Decompressor(StringRef Data);
 
-  Error consumeCompressedZLibHeader(bool Is64Bit, bool IsLittleEndian);
+  Error consumeCompressedSectionHeader(bool Is64Bit, bool IsLittleEndian);
 
   StringRef SectionData;
   uint64_t DecompressedSize;
+  compression::CompressionAlgorithm CompressionScheme;
 };
 
 } // end namespace object
 } // end namespace llvm
 
 #endif // LLVM_OBJECT_DECOMPRESSOR_H

llvm/include/llvm/Support/Compression.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_SUPPORT_COMPRESSION_H
 #define LLVM_SUPPORT_COMPRESSION_H
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/DataTypes.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/ErrorHandling.h"
 
 namespace llvm {
 template <typename T> class SmallVectorImpl;
 class Error;
 
 namespace compression {
-namespace zlib {
 
-constexpr int NoCompression = 0;
-constexpr int BestSpeedCompression = 1;
-constexpr int DefaultCompression = 6;
-constexpr int BestSizeCompression = 9;
-
-bool isAvailable();
+enum class SupportCompressionType : uint8_t {
+  Base = 255, ///< Abstract compression
+  None = 0,   ///< No compression
+  Zlib = 1,   ///< zlib style complession
+  ZStd = 2,   ///< zstd style complession
+};
+
+// This is the base class of all compression algorithms that llvm support
+// handles.
+class CompressionAlgorithm {
+public:
+  static constexpr SupportCompressionType AlgorithmId =
+      SupportCompressionType::Base;
+  static constexpr StringRef name = "base";
+  static constexpr int BestSpeedCompression = -999;
+  static constexpr int DefaultCompression = -999;
+  static constexpr int BestSizeCompression = -999;
+
+  // // estimates level to achive compression speed around scale*(speed at level
+  // which has max (speed*ratio) on mozilla-unified Bundle) int
+  // levelToTargetCompressionSpeed(float scale){
+  //   return 1;
+  // };
+  bool supported();
 
   void compress(ArrayRef<uint8_t> Input,
                 SmallVectorImpl<uint8_t> &CompressedBuffer,
                 int Level = DefaultCompression);
 
-Error uncompress(ArrayRef<uint8_t> Input, uint8_t *UncompressedBuffer,
+  Error decompress(ArrayRef<uint8_t> Input, uint8_t *UncompressedBuffer,
                    size_t &UncompressedSize);
 
-Error uncompress(ArrayRef<uint8_t> Input,
+  Error decompress(ArrayRef<uint8_t> Input,
                    SmallVectorImpl<uint8_t> &UncompressedBuffer,
-                 size_t UncompressedSize);
-
-} // End of namespace zlib
+                   size_t UncompressedSize) {
+    UncompressedBuffer.resize_for_overwrite(UncompressedSize);
+    Error E = decompress(Input, UncompressedBuffer.data(), UncompressedSize);
+    if (UncompressedSize < UncompressedBuffer.size())
+      UncompressedBuffer.truncate(UncompressedSize);
+    return E;
+  }
+  constexpr CompressionAlgorithm(){};
+};
+
+class NoneCompressionAlgorithm : public CompressionAlgorithm {
+  using super = CompressionAlgorithm;
+
+public:
+  constexpr static SupportCompressionType AlgorithmId =
+      SupportCompressionType::None;
+  constexpr static StringRef name = "none";
+  constexpr static int BestSpeedCompression = 0;
+  constexpr static int DefaultCompression = 0;
+  constexpr static int BestSizeCompression = 0;
 
-namespace zstd {
+  bool supported();
+  void compress(ArrayRef<uint8_t> Input,
+                SmallVectorImpl<uint8_t> &CompressedBuffer,
+                int Level = DefaultCompression);
 
-constexpr int NoCompression = -5;
-constexpr int BestSpeedCompression = 1;
-constexpr int DefaultCompression = 5;
-constexpr int BestSizeCompression = 12;
+  Error decompress(ArrayRef<uint8_t> Input, uint8_t *UncompressedBuffer,
+                   size_t &UncompressedSize);
 
-bool isAvailable();
+  Error decompress(ArrayRef<uint8_t> Input,
+                   SmallVectorImpl<uint8_t> &UncompressedBuffer,
+                   size_t UncompressedSize) {
+    UncompressedBuffer.resize_for_overwrite(UncompressedSize);
+    Error E = decompress(Input, UncompressedBuffer.data(), UncompressedSize);
+    if (UncompressedSize < UncompressedBuffer.size())
+      UncompressedBuffer.truncate(UncompressedSize);
+    return E;
+  }
+
+  constexpr NoneCompressionAlgorithm() : super(){};
+};
+
+class ZlibCompressionAlgorithm : public CompressionAlgorithm {
+  using super = CompressionAlgorithm;
+
+public:
+  constexpr static SupportCompressionType AlgorithmId =
+      SupportCompressionType::Zlib;
+  constexpr static StringRef name = "zlib";
+  constexpr static int BestSpeedCompression = 1;
+  constexpr static int DefaultCompression = 6;
+  constexpr static int BestSizeCompression = 9;
 
+  bool supported();
   void compress(ArrayRef<uint8_t> Input,
                 SmallVectorImpl<uint8_t> &CompressedBuffer,
                 int Level = DefaultCompression);
 
-Error uncompress(ArrayRef<uint8_t> Input, uint8_t *UncompressedBuffer,
+  Error decompress(ArrayRef<uint8_t> Input, uint8_t *UncompressedBuffer,
                    size_t &UncompressedSize);
 
-Error uncompress(ArrayRef<uint8_t> Input,
+  Error decompress(ArrayRef<uint8_t> Input,
                    SmallVectorImpl<uint8_t> &UncompressedBuffer,
-                 size_t UncompressedSize);
+                   size_t UncompressedSize) {
+    UncompressedBuffer.resize_for_overwrite(UncompressedSize);
+    Error E = decompress(Input, UncompressedBuffer.data(), UncompressedSize);
+    if (UncompressedSize < UncompressedBuffer.size())
+      UncompressedBuffer.truncate(UncompressedSize);
+    return E;
+  }
+  constexpr ZlibCompressionAlgorithm() : super(){};
+};
+
+class ZStdCompressionAlgorithm : public CompressionAlgorithm {
+  using super = CompressionAlgorithm;
+
+public:
+  constexpr static SupportCompressionType AlgorithmId =
+      SupportCompressionType::ZStd;
+  constexpr static StringRef name = "zstd";
+  constexpr static int BestSpeedCompression = 1;
+  constexpr static int DefaultCompression = 5;
+  constexpr static int BestSizeCompression = 12;
 
-} // End of namespace zstd
+  bool supported();
+  void compress(ArrayRef<uint8_t> Input,
+                SmallVectorImpl<uint8_t> &CompressedBuffer,
+                int Level = DefaultCompression);
+
+  Error decompress(ArrayRef<uint8_t> Input, uint8_t *UncompressedBuffer,
+                   size_t &UncompressedSize);
+
+  Error decompress(ArrayRef<uint8_t> Input,
+                   SmallVectorImpl<uint8_t> &UncompressedBuffer,
+                   size_t UncompressedSize) {
+    UncompressedBuffer.resize_for_overwrite(UncompressedSize);
+    Error E = decompress(Input, UncompressedBuffer.data(), UncompressedSize);
+    if (UncompressedSize < UncompressedBuffer.size())
+      UncompressedBuffer.truncate(UncompressedSize);
+    return E;
+  }
+  constexpr ZStdCompressionAlgorithm() : super(){};
+};
+llvm::compression::CompressionAlgorithm
+CompressionAlgorithmFromId(uint8_t CompressionSchemeId);
 
 } // End of namespace compression
 
 } // End of namespace llvm
 
 #endif

llvm/lib/MC/ELFObjectWriter.cpp

 
 void ELFWriter::writeSectionData(const MCAssembler &Asm, MCSection &Sec,
                                  const MCAsmLayout &Layout) {
   MCSectionELF &Section = static_cast<MCSectionELF &>(Sec);
   StringRef SectionName = Section.getName();
 
   auto &MC = Asm.getContext();
   const auto &MAI = MC.getAsmInfo();
-
-  bool CompressionEnabled =
-      MAI->compressDebugSections() != DebugCompressionType::None;
+  const DebugCompressionType CompressionType = MAI->compressDebugSections();
+  bool CompressionEnabled = CompressionType != DebugCompressionType::None;
   if (!CompressionEnabled || !SectionName.startswith(".debug_")) {
     Asm.writeSectionData(W.OS, &Section, Layout);
     return;
   }
 
-  assert(MAI->compressDebugSections() == DebugCompressionType::Z &&
+  assert(CompressionType == DebugCompressionType::Z &&
          "expected zlib style compression");
 
   SmallVector<char, 128> UncompressedData;
   raw_svector_ostream VecOS(UncompressedData);
   Asm.writeSectionData(VecOS, &Section, Layout);
 
   SmallVector<uint8_t, 128> Compressed;
   const uint32_t ChType = ELF::ELFCOMPRESS_ZLIB;
-  compression::zlib::compress(
+  compression::ZlibCompressionAlgorithm().compress(
       makeArrayRef(reinterpret_cast<uint8_t *>(UncompressedData.data()),
                    UncompressedData.size()),
       Compressed);
 
   if (!maybeWriteCompression(ChType, UncompressedData.size(), Compressed,
                              Sec.getAlignment())) {
     W.OS << UncompressedData;
     return;

llvm/lib/ObjCopy/ELF/ELFObject.cpp

Error SectionWriter::visit(const OwnedDataSection &Sec) {
   return Error::success();
 }
 
 template <class ELFT>
 Error ELFSectionWriter<ELFT>::visit(const DecompressedSection &Sec) {
   ArrayRef<uint8_t> Compressed =
       Sec.OriginalData.slice(sizeof(Elf_Chdr_Impl<ELFT>));
   SmallVector<uint8_t, 128> DecompressedContent;
-  if (Error Err = compression::zlib::uncompress(Compressed, DecompressedContent,
-                                                static_cast<size_t>(Sec.Size)))
+  DebugCompressionType CompressionType =
+      reinterpret_cast<const Elf_Chdr_Impl<ELFT> *>(Sec.OriginalData.data())
+                  ->ch_type == ELF::ELFCOMPRESS_ZLIB
+          ? DebugCompressionType::Z
+          : DebugCompressionType::None;
+
+  switch (CompressionType) {
+  case DebugCompressionType::Z:
+    if (Error Err1 = compression::ZlibCompressionAlgorithm().decompress(
+            Compressed, DecompressedContent, static_cast<size_t>(Sec.Size))) {
       return createStringError(errc::invalid_argument,
-                             "'" + Sec.Name + "': " + toString(std::move(Err)));
+                               "'" + Sec.Name +
+                                   "': " + toString(std::move(Err1)));
+    }
+    break;
+  case DebugCompressionType::None:
+    llvm_unreachable("unexpected DebugCompressionType::None");
+    break;
+  }
 
   uint8_t *Buf = reinterpret_cast<uint8_t *>(Out.getBufferStart()) + Sec.Offset;
   std::copy(DecompressedContent.begin(), DecompressedContent.end(), Buf);
 
   return Error::success();
 }
 
 Error BinarySectionWriter::visit(const DecompressedSection &Sec) {
   return createStringError(errc::operation_not_permitted,
Error ELFSectionWriter<ELFT>::visit(const CompressedSection &Sec) {
   std::copy(Sec.CompressedData.begin(), Sec.CompressedData.end(), Buf);
   return Error::success();
 }
 
 CompressedSection::CompressedSection(const SectionBase &Sec,
                                      DebugCompressionType CompressionType)
     : SectionBase(Sec), CompressionType(CompressionType),
       DecompressedSize(Sec.OriginalData.size()), DecompressedAlign(Sec.Align) {
-  compression::zlib::compress(OriginalData, CompressedData);
+  switch (CompressionType) {
+  case DebugCompressionType::Z:
+    compression::ZlibCompressionAlgorithm().compress(OriginalData,
+                                                     CompressedData);
+    break;
+  case DebugCompressionType::None:
+    break;
+  }
 
   assert(CompressionType != DebugCompressionType::None);
   Flags |= ELF::SHF_COMPRESSED;
   size_t ChdrSize =
       std::max(std::max(sizeof(object::Elf_Chdr_Impl<object::ELF64LE>),
                         sizeof(object::Elf_Chdr_Impl<object::ELF64BE>)),
                std::max(sizeof(object::Elf_Chdr_Impl<object::ELF32LE>),
                         sizeof(object::Elf_Chdr_Impl<object::ELF32BE>)));
   Size = ChdrSize + CompressedData.size();

llvm/lib/Object/Decompressor.cpp

 #include "llvm/Support/Endian.h"
 
 using namespace llvm;
 using namespace llvm::support::endian;
 using namespace object;
 
 Expected<Decompressor> Decompressor::create(StringRef Name, StringRef Data,
                                             bool IsLE, bool Is64Bit) {
-  if (!compression::zlib::isAvailable())
-    return createError("zlib is not available");
 
   Decompressor D(Data);
-  if (Error Err = D.consumeCompressedZLibHeader(Is64Bit, IsLE))
+  if (Error Err = D.consumeCompressedSectionHeader(Is64Bit, IsLE))
     return std::move(Err);
   return D;
 }
 
 Decompressor::Decompressor(StringRef Data)
     : SectionData(Data), DecompressedSize(0) {}
 
-Error Decompressor::consumeCompressedZLibHeader(bool Is64Bit,
+Error Decompressor::consumeCompressedSectionHeader(bool Is64Bit,
                                                    bool IsLittleEndian) {
   using namespace ELF;
   uint64_t HdrSize = Is64Bit ? sizeof(Elf64_Chdr) : sizeof(Elf32_Chdr);
   if (SectionData.size() < HdrSize)
     return createError("corrupted compressed section header");
 
   DataExtractor Extractor(SectionData, IsLittleEndian, 0);
   uint64_t Offset = 0;
-  if (Extractor.getUnsigned(&Offset, Is64Bit ? sizeof(Elf64_Word)
-                                             : sizeof(Elf32_Word)) !=
-      ELFCOMPRESS_ZLIB)
+  uint64_t ELFCompressionSchemeId = Extractor.getUnsigned(
+      &Offset, Is64Bit ? sizeof(Elf64_Word) : sizeof(Elf32_Word));
+  if (ELFCompressionSchemeId == ELFCOMPRESS_ZLIB) {
+    CompressionScheme = compression::ZlibCompressionAlgorithm();
+  } else {
     return createError("unsupported compression type");
+  }
+  if (!CompressionScheme.supported())
+    return createError(CompressionScheme.name + " is not available");
 
   // Skip Elf64_Chdr::ch_reserved field.
   if (Is64Bit)
     Offset += sizeof(Elf64_Word);
 
   DecompressedSize = Extractor.getUnsigned(
       &Offset, Is64Bit ? sizeof(Elf64_Xword) : sizeof(Elf32_Word));
   SectionData = SectionData.substr(HdrSize);
   return Error::success();
 }
 
 Error Decompressor::decompress(MutableArrayRef<uint8_t> Buffer) {
   size_t Size = Buffer.size();
-  return compression::zlib::uncompress(arrayRefFromStringRef(SectionData),
-                                       Buffer.data(), Size);
+  return CompressionScheme.decompress(arrayRefFromStringRef(SectionData),
 }

llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp

Error RawCoverageFilenamesReader::read(CovMapVersion Version) {
   if (auto Err = readULEB128(UncompressedLen))
     return Err;
 
   uint64_t CompressedLen;
   if (auto Err = readSize(CompressedLen))
     return Err;
 
   if (CompressedLen > 0) {
-    if (!compression::zlib::isAvailable())
+    if (!compression::ZlibCompressionAlgorithm().supported())
       return make_error<CoverageMapError>(
           coveragemap_error::decompression_failed);
 
     // Allocate memory for the decompressed filenames.
     SmallVector<uint8_t, 0> StorageBuf;
 
     // Read compressed filenames.
     StringRef CompressedFilenames = Data.substr(0, CompressedLen);
     Data = Data.substr(CompressedLen);
-    auto Err = compression::zlib::uncompress(
+    auto Err = compression::ZlibCompressionAlgorithm().decompress(
         arrayRefFromStringRef(CompressedFilenames), StorageBuf,
         UncompressedLen);
     if (Err) {
       consumeError(std::move(Err));
       return make_error<CoverageMapError>(
           coveragemap_error::decompression_failed);
     }
 

llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp

std::string FilenamesStr;
     raw_string_ostream FilenamesOS{FilenamesStr};
     for (const auto &Filename : Filenames) {
       encodeULEB128(Filename.size(), FilenamesOS);
       FilenamesOS << Filename;
     }
   }
 
   SmallVector<uint8_t, 128> CompressedStr;
-  bool doCompression = Compress && compression::zlib::isAvailable() &&
-                       DoInstrProfNameCompression;
+  compression::CompressionAlgorithm CompressionScheme =
+      compression::ZlibCompressionAlgorithm();
+  bool doCompression =
+      Compress && CompressionScheme.supported() && DoInstrProfNameCompression;
   if (doCompression)
-    compression::zlib::compress(arrayRefFromStringRef(FilenamesStr),
-                                CompressedStr,
-                                compression::zlib::BestSizeCompression);
+    CompressionScheme.compress(arrayRefFromStringRef(FilenamesStr),
+                               CompressionScheme.BestSizeCompression);
 
   // ::= <num-filenames>
   //     <uncompressed-len>
   //     <compressed-len-or-zero>
   //     (<compressed-filenames> | <uncompressed-filenames>)
   encodeULEB128(Filenames.size(), OS);
   encodeULEB128(FilenamesStr.size(), OS);
   encodeULEB128(doCompression ? CompressedStr.size() : 0U, OS);
   OS << (doCompression ? toStringRef(CompressedStr) : StringRef(FilenamesStr));
 }
 
 namespace {
 
 /// Gather only the expressions that are used by the mapping

llvm/lib/ProfileData/InstrProf.cpp

auto WriteStringToResult = [&](size_t CompressedLen, StringRef InputStr) {
     P += EncLen;
     char *HeaderStr = reinterpret_cast<char *>(&Header[0]);
     unsigned HeaderLen = P - &Header[0];
     Result.append(HeaderStr, HeaderLen);
     Result += InputStr;
     return Error::success();
   };
 
   if (!doCompression) {
     return WriteStringToResult(0, UncompressedNameStrings);
   }
-
+  compression::CompressionAlgorithm CompressionScheme =
+      compression::ZlibCompressionAlgorithm();
   SmallVector<uint8_t, 128> CompressedNameStrings;
-  compression::zlib::compress(arrayRefFromStringRef(UncompressedNameStrings),
-                              CompressedNameStrings,
-                              compression::zlib::BestSizeCompression);
+  CompressionScheme.compress(arrayRefFromStringRef(UncompressedNameStrings),
+                             CompressionScheme.BestSizeCompression);
 
   return WriteStringToResult(CompressedNameStrings.size(),
                              toStringRef(CompressedNameStrings));
 }
 
 StringRef getPGOFuncNameVarInitializer(GlobalVariable *NameVar) {
   auto *Arr = cast<ConstantDataArray>(NameVar->getInitializer());
   StringRef NameStr =
       Arr->isCString() ? Arr->getAsCString() : Arr->getAsString();
   return NameStr;
 }
 
 Error collectPGOFuncNameStrings(ArrayRef<GlobalVariable *> NameVars,
                                 std::string &Result, bool doCompression) {
   std::vector<std::string> NameStrs;
   for (auto *NameVar : NameVars) {
     NameStrs.push_back(std::string(getPGOFuncNameVarInitializer(NameVar)));
   }
+  compression::CompressionAlgorithm CompressionScheme =
+      compression::ZlibCompressionAlgorithm();
   return collectPGOFuncNameStrings(
-      NameStrs, compression::zlib::isAvailable() && doCompression, Result);
+      NameStrs, CompressionScheme.supported() && doCompression, Result);
 }
 
 Error readPGOFuncNameStrings(StringRef NameStrings, InstrProfSymtab &Symtab) {
+  compression::CompressionAlgorithm CompressionScheme =
+      compression::ZlibCompressionAlgorithm();
+
   const uint8_t *P = NameStrings.bytes_begin();
   const uint8_t *EndP = NameStrings.bytes_end();
   while (P < EndP) {
     uint32_t N;
     uint64_t UncompressedSize = decodeULEB128(P, &N);
     P += N;
     uint64_t CompressedSize = decodeULEB128(P, &N);
     P += N;
     bool isCompressed = (CompressedSize != 0);
     SmallVector<uint8_t, 128> UncompressedNameStrings;
     StringRef NameStrings;
     if (isCompressed) {
-      if (!llvm::compression::zlib::isAvailable())
+      if (!CompressionScheme.supported())
         return make_error<InstrProfError>(instrprof_error::zlib_unavailable);
 
-      if (Error E = compression::zlib::uncompress(
+      if (Error E = CompressionScheme.decompress(
               makeArrayRef(P, CompressedSize), UncompressedNameStrings,
               UncompressedSize)) {
         consumeError(std::move(E));
         return make_error<InstrProfError>(instrprof_error::uncompress_failed);
       }
       P += CompressedSize;
       NameStrings = toStringRef(UncompressedNameStrings);
     } else {

llvm/lib/ProfileData/SampleProfReader.cpp

std::error_code SampleProfileReaderExtBinaryBase::decompressSection(
   if (std::error_code EC = DecompressSize.getError())
     return EC;
   DecompressBufSize = *DecompressSize;
 
   auto CompressSize = readNumber<uint64_t>();
   if (std::error_code EC = CompressSize.getError())
     return EC;
 
-  if (!llvm::compression::zlib::isAvailable())
+  if (!llvm::compression::ZlibCompressionAlgorithm().supported())
     return sampleprof_error::zlib_unavailable;
 
   uint8_t *Buffer = Allocator.Allocate<uint8_t>(DecompressBufSize);
   size_t UCSize = DecompressBufSize;
-  llvm::Error E = compression::zlib::uncompress(
+  llvm::Error E = compression::ZlibCompressionAlgorithm().decompress(
       makeArrayRef(Data, *CompressSize), Buffer, UCSize);
   if (E)
     return sampleprof_error::uncompress_failed;
   DecompressBuf = reinterpret_cast<const uint8_t *>(Buffer);
   return sampleprof_error::success;
 }
 
 std::error_code SampleProfileReaderExtBinaryBase::readImpl() {

llvm/lib/ProfileData/SampleProfWriter.cpp

SampleProfileWriterExtBinaryBase::markSectionStart(SecType Type,
   assert(Entry.Type == Type && "Unexpected section type");
   // Use LocalBuf as a temporary output for writting data.
   if (hasSecFlag(Entry, SecCommonFlags::SecFlagCompress))
     LocalBufStream.swap(OutputStream);
   return SectionStart;
 }
 
 std::error_code SampleProfileWriterExtBinaryBase::compressAndOutput() {
-  if (!llvm::compression::zlib::isAvailable())
+  compression::CompressionAlgorithm CompressionScheme =
+      compression::ZlibCompressionAlgorithm();
+  if (!CompressionScheme.supported())
     return sampleprof_error::zlib_unavailable;
   std::string &UncompressedStrings =
       static_cast<raw_string_ostream *>(LocalBufStream.get())->str();
   if (UncompressedStrings.size() == 0)
     return sampleprof_error::success;
   auto &OS = *OutputStream;
   SmallVector<uint8_t, 128> CompressedStrings;
-  compression::zlib::compress(arrayRefFromStringRef(UncompressedStrings),
-                              CompressedStrings,
-                              compression::zlib::BestSizeCompression);
+  CompressionScheme.compress(arrayRefFromStringRef(UncompressedStrings),
+                             CompressionScheme.BestSizeCompression);
   encodeULEB128(UncompressedStrings.size(), OS);
   encodeULEB128(CompressedStrings.size(), OS);
   OS << toStringRef(CompressedStrings);
   UncompressedStrings.clear();
   return sampleprof_error::success;
 }
 
 /// Add a new section into section header table given the section type

llvm/lib/Support/Compression.cpp

 #endif
 #if LLVM_ENABLE_ZSTD
 #include <zstd.h>
 #endif
 
 using namespace llvm;
 using namespace llvm::compression;
 
+constexpr SupportCompressionType CompressionAlgorithm::AlgorithmId;
+constexpr StringRef CompressionAlgorithm::name;
+constexpr int CompressionAlgorithm::BestSpeedCompression;
+constexpr int CompressionAlgorithm::DefaultCompression;
+constexpr int CompressionAlgorithm::BestSizeCompression;
+
+bool CompressionAlgorithm::supported() { return false; };
+
+void CompressionAlgorithm::compress(ArrayRef<uint8_t> Input,
+                                    SmallVectorImpl<uint8_t> &CompressedBuffer,
+                                    int Level) {
+  llvm_unreachable("method:\"compress\" is unsupported for compression "
+                   "algorithm:\"base\", reason:\"can't call on base\"");
+};
+Error CompressionAlgorithm::decompress(ArrayRef<uint8_t> Input,
+                                       uint8_t *UncompressedBuffer,
+                                       size_t &UncompressedSize) {
+  llvm_unreachable("method:\"decompress\" is unsupported for compression "
+                   "algorithm:\"base\", reason:\"can't call on base\"");
+}
+
+constexpr SupportCompressionType NoneCompressionAlgorithm::AlgorithmId;
+constexpr StringRef NoneCompressionAlgorithm::name;
+constexpr int NoneCompressionAlgorithm::BestSpeedCompression;
+constexpr int NoneCompressionAlgorithm::DefaultCompression;
+constexpr int NoneCompressionAlgorithm::BestSizeCompression;
+
+bool NoneCompressionAlgorithm::supported() { return true; };
+
+void NoneCompressionAlgorithm::compress(
+    ArrayRef<uint8_t> Input, SmallVectorImpl<uint8_t> &CompressedBuffer,
+    int Level) {
+  unsigned long CompressedSize = Input.size();
+  CompressedBuffer.resize_for_overwrite(CompressedSize);
+  // SmallVectorImpl<uint8_t>()
+  CompressedBuffer.assign(SmallVector<uint8_t>(Input.begin(), Input.end()));
+
+  // Tell MemorySanitizer that zlib output buffer is fully initialized.
+  // This avoids a false report when running LLVM with uninstrumented ZLib.
+  __msan_unpoison(CompressedBuffer.data(), CompressedSize);
+  if (CompressedSize < CompressedBuffer.size())
+    CompressedBuffer.truncate(CompressedSize);
+};
+Error NoneCompressionAlgorithm::decompress(ArrayRef<uint8_t> Input,
+                                           uint8_t *UncompressedBuffer,
+                                           size_t &UncompressedSize) {
+  // Tell MemorySanitizer that zlib output buffer is fully initialized.
+  // This avoids a false report when running LLVM with uninstrumented ZLib.
+  if (UncompressedSize < Input.size()) {
+    return make_error<StringError>("decompressed buffer target size too small",
+                                   inconvertibleErrorCode());
+  }
+  UncompressedSize = Input.size();
+  memcpy(UncompressedBuffer, Input.data(), UncompressedSize);
+
+  __msan_unpoison(UncompressedBuffer, UncompressedSize);
+  return Error::success();
+}
+
+constexpr SupportCompressionType ZlibCompressionAlgorithm::AlgorithmId;
+constexpr StringRef ZlibCompressionAlgorithm::name;
+constexpr int ZlibCompressionAlgorithm::BestSpeedCompression;
+constexpr int ZlibCompressionAlgorithm::DefaultCompression;
+constexpr int ZlibCompressionAlgorithm::BestSizeCompression;
+
 #if LLVM_ENABLE_ZLIB
 
 static StringRef convertZlibCodeToString(int Code) {
   switch (Code) {
   case Z_MEM_ERROR:
     return "zlib error: Z_MEM_ERROR";
   case Z_BUF_ERROR:
     return "zlib error: Z_BUF_ERROR";
   case Z_STREAM_ERROR:
     return "zlib error: Z_STREAM_ERROR";
   case Z_DATA_ERROR:
     return "zlib error: Z_DATA_ERROR";
   case Z_OK:
   default:
     llvm_unreachable("unknown or unexpected zlib status code");
   }
 }
 
-bool zlib::isAvailable() { return true; }
+bool ZlibCompressionAlgorithm::supported() { return true; };
 
-void zlib::compress(ArrayRef<uint8_t> Input,
-                    SmallVectorImpl<uint8_t> &CompressedBuffer, int Level) {
+void ZlibCompressionAlgorithm::compress(
+    ArrayRef<uint8_t> Input, SmallVectorImpl<uint8_t> &CompressedBuffer,
+    int Level) {
   unsigned long CompressedSize = ::compressBound(Input.size());
   CompressedBuffer.resize_for_overwrite(CompressedSize);
   int Res = ::compress2((Bytef *)CompressedBuffer.data(), &CompressedSize,
                         (const Bytef *)Input.data(), Input.size(), Level);
   if (Res == Z_MEM_ERROR)
     report_bad_alloc_error("Allocation failed");
   assert(Res == Z_OK);
   // Tell MemorySanitizer that zlib output buffer is fully initialized.
   // This avoids a false report when running LLVM with uninstrumented ZLib.
   __msan_unpoison(CompressedBuffer.data(), CompressedSize);
   if (CompressedSize < CompressedBuffer.size())
     CompressedBuffer.truncate(CompressedSize);
-}
-
-Error zlib::uncompress(ArrayRef<uint8_t> Input, uint8_t *UncompressedBuffer,
+};
+Error ZlibCompressionAlgorithm::decompress(ArrayRef<uint8_t> Input,
+                                           uint8_t *UncompressedBuffer,
                                            size_t &UncompressedSize) {
   int Res =
       ::uncompress((Bytef *)UncompressedBuffer, (uLongf *)&UncompressedSize,
                    (const Bytef *)Input.data(), Input.size());
   // Tell MemorySanitizer that zlib output buffer is fully initialized.
   // This avoids a false report when running LLVM with uninstrumented ZLib.
   __msan_unpoison(UncompressedBuffer, UncompressedSize);
   return Res ? make_error<StringError>(convertZlibCodeToString(Res),
                                        inconvertibleErrorCode())
              : Error::success();
-}
-
-Error zlib::uncompress(ArrayRef<uint8_t> Input,
-                       SmallVectorImpl<uint8_t> &UncompressedBuffer,
-                       size_t UncompressedSize) {
-  UncompressedBuffer.resize_for_overwrite(UncompressedSize);
-  Error E =
-      zlib::uncompress(Input, UncompressedBuffer.data(), UncompressedSize);
-  if (UncompressedSize < UncompressedBuffer.size())
-    UncompressedBuffer.truncate(UncompressedSize);
-  return E;
-}
+};
 
 #else
-bool zlib::isAvailable() { return false; }
-void zlib::compress(ArrayRef<uint8_t> Input,
-                    SmallVectorImpl<uint8_t> &CompressedBuffer, int Level) {
-  llvm_unreachable("zlib::compress is unavailable");
-}
-Error zlib::uncompress(ArrayRef<uint8_t> Input, uint8_t *UncompressedBuffer,
-                       size_t &UncompressedSize) {
-  llvm_unreachable("zlib::uncompress is unavailable");
-}
-Error zlib::uncompress(ArrayRef<uint8_t> Input,
+bool ZlibCompressionAlgorithm::supported() { return false; };
+
+void ZlibCompressionAlgorithm::compress(
+    ArrayRef<uint8_t> Input, SmallVectorImpl<uint8_t> &CompressedBuffer,
+    int Level) {
+  llvm_unreachable(
+      "method:\"compress\" is unsupported for compression algorithm:\"zlib\", "
+      "reason:\"llvm not compiled with zlib support\"");
+};
+Error ZlibCompressionAlgorithm::decompress(ArrayRef<uint8_t> Input,