[PATCH] D36346: Fix thinlto cache key computation for cfi-icall.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: llvm/lib/LTO/LTO.cpp:233
+  // that are referenced or defined in this module.
+  DenseMap<GlobalValue::GUID, bool> UsedCfiDefs;
+  DenseMap<GlobalValue::GUID, bool> UsedCfiDecls;
----------------
We are recomputing this map (and the one below) once for each input file, which would seem to create O(NM) complexity. Can we avoid that by moving the map computation into the InProcessThinBackend? See for example how we handle TypeIdSummariesByGuid.


================
Comment at: llvm/lib/LTO/LTO.cpp:256
+
+  for (auto &GSM : DefinedGlobals) {
+    AddUsedCfiGlobal(GSM.first);
----------------
I think you can move the body of this loop into AddTypeIdSummary (after giving it a better name).


================
Comment at: llvm/lib/LTO/LTO.cpp:258
+    AddUsedCfiGlobal(GSM.first);
+    for (const ValueInfo &VI : GSM.second->refs())
+      AddUsedCfiGlobal(VI.getGUID());
----------------
I think you also need to include calls().


================
Comment at: llvm/test/ThinLTO/X86/Inputs/cache-icall-no.ll:1
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
----------------
Do you need this file as part of your test case? I think you just need to *not* provide an LTO definition of f. That's basically what the reproducer in the PR is doing.


Repository:
  rL LLVM

https://reviews.llvm.org/D36346