[PATCH] D53773: [ExecutionEngine] Track objects using an abstract ObjectKey in JITEventListeners.

[Lang Hames via Phabricator via llvm-commits]

lhames added a comment.

In https://reviews.llvm.org/D53773#1280967, @anarazel wrote:

> This looks like a sensible improvement. There's probably not many implementors of the interface, so the renaming shouldn't cause much pain.
>
> I noticed however that you didn't update ExecutionEngine/PerfJITEvents/PerfJITEventListener.cpp?


Oops -- I totally missed that. I will fix it and post an updated patch tomorrow.



================
Comment at: lib/ExecutionEngine/MCJIT/MCJIT.cpp:653
+                               const RuntimeDyld::LoadedObjectInfo &L) {
+  uint64_t Key =
+      static_cast<uint64_t>(reinterpret_cast<uintptr_t>(Obj.getData().data()));
----------------
anarazel wrote:
> Perhaps this should rather live inside ObjectFile? No strong opinion, just pondering. Seems a bit prettier than having the ugly multiple cast in multiple places.
Unfortunately this can not be moved to ObjectFile: For MCJIT it makes sense to continue using the original ObjectFile address as the key, but for ORC the aim is to decouple the key from the ObjectFile so that the latter can be deallocated.


Repository:
  rL LLVM

https://reviews.llvm.org/D53773