[PATCH] D46478: [Coverage] Take filenames into account when loading function records.

Max Moroz via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 8 11:36:21 PDT 2018


Dor1s marked an inline comment as done.
Dor1s added inline comments.


================
Comment at: lib/ProfileData/Coverage/CoverageMapping.cpp:224
+      return Error::success();
+  }
 
----------------
vsk wrote:
> Given that this should just be an optimization, it might be cheaper/simpler to use a map from the hash of the record's filenames to a set of stripped function names. This would preserve the proposed behavior where a record which introduces a duplicate symbol in at least one previously-unseen file is still loaded. Wdyt? I imagine something like `AlreadyLoaded = !RecordProvenance[hash_combine_range(Record.Filenames)].insert(OrigFuncName).second`. Hashing `OrigFuncName` might decrease the storage requirements even further.
Thanks for the hints, Vedant! That sounds better to me. Please take a look at the updated CL, what do you think about it?


================
Comment at: unittests/ProfileData/CoverageMappingTest.cpp:875
+
+  // This record should be loaded.
+  startFunction("func", 0x1234);
----------------
I've been confused by this behavior at first, but then realized that it's probably right, as this record must be something different than the one previously loaded at line 866


Repository:
  rL LLVM

https://reviews.llvm.org/D46478





More information about the llvm-commits mailing list