[PATCH] D154680: [llvm-exegesis] Make SubprocessMemoryTest use PIDs

[Aiden Grossman via Phabricator via llvm-commits]

aidengrossman added inline comments.


================
Comment at: llvm/unittests/tools/llvm-exegesis/X86/SubprocessMemoryTest.cpp:42
+                                  const unsigned DefinitionNumber) {
+    return "/" + std::to_string(getpid() * TestNumber) + "memdef" +
+           std::to_string(DefinitionNumber);
----------------
bjope wrote:
> bjope wrote:
> > Multiplying a pid with a number seems a bit weird to me. It is not really guaranteed to result in a number unique to the process (pid=1 and TestNumber=2 gives the same value as pid=2 and TestNumber=1). And I don't know if this multiplication potentially could overflow as well.
> Any news about this? Do you agree that the multiplication is weird?
Sorry for the delay. Planning on working on this over the weekend. I do agree that multiplication is weird the way I have it implemented here. However, I do think it's the most elegant solution available without changing the library code just to expose more testing hooks. I think doing 2D array indexing should work. We can do `getpid() * NumberOfTests + TestNumber` which should make everything unique and also won't overflow on 32 bit arithmetic as PIDs are limited to 2^22.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154680