[PATCH] D58718: [Memory] Add basic support for large/huge memory pages

[Alexandre Ganea via Phabricator via llvm-commits]

aganea marked 2 inline comments as done.
aganea added inline comments.


================
Comment at: lib/Support/Windows/Memory.inc:72
+      TP.Privileges[0].Attributes = SE_PRIVILEGE_ENABLED;
+      if (AdjustTokenPrivileges(Token, FALSE, &TP, 0, 0, 0)) {
+        DWORD E = GetLastError();
----------------
zturner wrote:
> zturner wrote:
> > Why do we actually even need to set this privilege on the process token?  Will huge pages not work without it?
> > 
> > Also, it's strange to be doing this from a function called `getLargePageSize` which sounds like it doesn't modify anything.
> Actually I found this that explains it:
> 
> https://blogs.msdn.microsoft.com/oldnewthing/20110128-00/?p=11643
> 
> Is looking up the privilege value and opening the token expensive?  If so, maybe you want to put this behind an `llvm::call_once()`
`getLargePageSize()` returns into a static variable (see L114) which shall be thread-safe as per [[ http://www.open-std.org/Jtc1/sc22/wg21/docs/papers/2014/n4296.pdf | C++11 specification ]] section 6.7.4. I know there were issues previously with VS2012, but any modern C++11/14 compiler should support that (I know for sure it works since VS2015).

Is there any reason for calling `llvm::call_once()` over `static` initialization in this case?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58718