[PATCH] D63466: [Remarks] Add an LLVM-bitstream-based remark serializer

[Francis Visoiu Mistrih via Phabricator via llvm-commits]

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


================
Comment at: llvm/include/llvm/Remarks/BitstreamRemarkContainer.h:32-34
+/// * separate: the metadata is separate from the remarks and points to the
+///             auxiliary file that contains the remarks.
+/// * standalone: the metadata and the remarks are emitted together.
----------------
paquette wrote:
> A couple things:
> 
> 1) Is remark metadata explicitly documented anywhere? The comment in BitstreamRemarkSerializer.h seems to be about half way there. Maybe it would be good to expand a bit upon that, and then reference that file here? (I think it might be worth adding to the LLVM docs on remarks eventually.)
> 
> 2) The "separate" point only talks about `SeparateRemarksFile`?
> 
> 
> 
1. Yes, I documented it right above the `SeparateRemarksMeta` enum entry. I agree that I should add something to the LLVM docs.
2. Actually, it only talks about `SeparateRemarksMeta`. I will make this more explicit, but the idea for the separate model is:
  * the meta part will eventually go into a section. It will contain one BLOCK_META with some records.
  * the remark part will go into a separate file (with its path saved in the meta part). It will also contain one BLOCK_META (for more versioning) and multiple BLOCK_REMARK blocks.


================
Comment at: llvm/include/llvm/Remarks/BitstreamRemarkSerializer.h:62-63
+  /// Note: depending on the container type, some IDs might be uninitialized.
+  /// Warning: When adding more abbrev IDs, make sure to update the
+  /// BlockCodeSize (in the call to EnterSubblock).
+  uint64_t RecordMetaContainerInfoAbbrevID = 0;
----------------
paquette wrote:
> Is it possible to verify this with a test?
> 
> E.g. add a debug dumper for the available abbrev IDs and do something similar to llvm/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir?
Nothing obvious comes to mind right now for iterating on the available abbrev IDs without clobbering the code with debug stuff. Technically, the BitstreamWriter will assert if the abbrev id's width is bigger than what specified when entering the block.


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

https://reviews.llvm.org/D63466