[PATCH] D29734: IR: Function summary extensions for whole-program devirtualization pass.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: llvm/include/llvm/IR/ModuleSummaryIndex.h:256
+  struct VFuncId {
+    GlobalValue::GUID GUID;
+    uint64_t Offset;
----------------
tejohnson wrote:
> I wonder if GUID should be moved out of GlobalValue since we are now using it for types?
Not sure -- it isn't clear where else it should go though. One possibility is ModuleSummaryIndex, but that will also require moving getGUID to avoid a circular dependency. Moving it should probably be a separate cleanup, I reckon.


================
Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:3371
 
+static void writeFunctionTypeMetadataRecords(BitstreamWriter &Stream,
+                                             FunctionSummary *FS) {
----------------
tejohnson wrote:
> tejohnson wrote:
> > Why no abbrev ids for these record types?
> Add comments to new static funcs here and elsewhere
> Why no abbrev ids for these record types?
It isn't clear that it would be worth it with the current abbrev scheme. VBR would be a good choice for offsets and arguments since they are typically relatively small, but GUIDs are evenly distributed so the best choice for those would be Fixed.

So a good abbrev for *_VCALLS would involve an array of (Fixed, VBR) pairs, but I don't think we can express that at the moment. And a good abbrev for *_CONST_VCALL would use an array of VBR. But that is basically just the encoding we use at the moment for unabbreviated records.


https://reviews.llvm.org/D29734