[PATCH] D131992: [Support] compression proposal for a enum->spec->impl approach

[Cole Kissane via Phabricator via llvm-commits]

ckissane marked 2 inline comments as done.
ckissane added inline comments.


================
Comment at: llvm/include/llvm/Support/Compression.h:38-39
+
+CompressionSpecRef getCompressionSpec(uint8_t Kind);
+CompressionSpecRef getCompressionSpec(CompressionKind Kind);
+CompressionSpecRef getSchemeDetails(CompressionImplRef Implementation);
----------------
dblaikie wrote:
> Probably don't need both of these, just the one that takes the enum type?
removed one


================
Comment at: llvm/lib/ProfileData/InstrProf.cpp:495
   return collectPGOFuncNameStrings(
-      NameStrs, compression::zlib::isAvailable() && doCompression, Result);
+      NameStrs, doCompression ? OptionalCompressionScheme : nullptr, Result);
 }
----------------
dblaikie wrote:
> looks like this could be changed to pass the implementation, without the spec? (the caller doesn't need/use the spec)
`(the caller doesn't need/use the spec)`...
It definitely might in the future because it writes a header, that (in the conceivably near future) (like one of my follow up patches) could contain the uint 8 value of the compression type from the Spec, and 0 if uncompressed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131992/new/

https://reviews.llvm.org/D131992