[PATCH] D60242: Add IR support, ELF section and user documentation for partitioning feature.

Peter Collingbourne via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 28 20:27:31 PDT 2019


pcc marked 7 inline comments as done.
pcc added a comment.

Thanks for the review. Committed rL361922 <https://reviews.llvm.org/rL361922> (rename type allocator) and rL361923 <https://reviews.llvm.org/rL361923> (this change).



================
Comment at: llvm/lib/Bitcode/Reader/BitcodeReader.cpp:2986
 
+  if (Record.size() > 15)
+    NewGV->setPartition(StringRef(Strtab.data() + Record[14], Record[15]));
----------------
echristo wrote:
> Comment what the conditional is checking please?
I added some comments to this file, but I'm not sure how useful they are. Maybe I'm biased from working on this code before, but (aside from the additional comment I added in `parseFunctionRecord`) I think the code speaks for itself here and I'm not sure if my comment is really adding any useful information. If we added similar comments elsewhere in these functions for consistency, I think that would result in less information per line of code which can hurt readability. Can you see a better comment to add here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60242





More information about the llvm-commits mailing list