[PATCH] D146238: [StandardInstrumentations] Check that module analyses are properly invalidated

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added inline comments.


================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:1126
       FAM.registerPass([&] { return PreservedFunctionHashAnalysis(); });
+      MAM.registerPass([&] { return PreservedModuleHashAnalysis(); });
       Registered = true;
----------------
nikic wrote:
> Do this outside the pass callback?
might as well be consistent with the `FAM` code, which requires having some IR to get from the MAM (or we coul pass in the `FAM`, but that's more work for callers)


================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:1204
+      }
+    }
   });
----------------
nikic wrote:
> Hm, does that mean that in most cases we're going to verify both all functions via the function hash, and then again using the module hash (which includes all function hashes)?
yes, but that's fine, it's only doubling this work (which shouldn't matter in expensive checks builds)

we could have the module analysis fetch the hash of each function via the function hash analysis, but at that point we start mixing module and function analyses which I'd like to avoid to keep this conceptually simple and not fall into traps with cross-level analyses (and it would require changes of how we hash modules by combining function hashes rather than hashing everything one after the other, but that's not too much work)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146238