[PATCH] D71975: [Support] Support MF_HUGE_HINT on Linux and FreeBSD

[Bruno Ricci via Phabricator via llvm-commits]

riccibruno planned changes to this revision.
riccibruno marked 2 inline comments as done.
riccibruno added a comment.

In D71975#1799355 <https://reviews.llvm.org/D71975#1799355>, @rnk wrote:

> + at aganea, who added this flag.
>
> > Note that only a little test is added since the allocateMappedMemory interface is already well-tested. It seems difficult to reliably test that we got large pages. I am open to suggestions here (maybe with userfaultfd ?).
>
> At a certain point, this would really be a test for the OS. That's pretty hard. I think this is fine without additional testing.


So I did rework the patch to address your comments, but I found out an "interesting" design decision with how `mmap`/`munmap` work on linux:

- `mmap` with the `MAP_HUGETLB` flag automatically adjusts the `length` parameter to be a multiple of the size of a huge page. But there is no easy way to find out to what value the length was adjusted to. There is also no easy way to find what the size of a huge page is. The recommended method (and what `libhugetlbfs` does) is to parse `/proc/meminfo`.
- `munmap` requires the adjusted length.

So I need to:

- Add a `getHugePageSize` similar to `sys::Process::getPageSize` which will do its best to find out about the huge page size(s) on the platform. This needs to be done for Windows too.
- Use that in `allocateMappedMemory`.



================
Comment at: llvm/lib/Support/Unix/Memory.inc:169
+    if (PFlags & llvm::sys::Memory::MF_HUGE_HINT)
+      return allocateMappedMemoryImpl(NumBytes, NearBlock,
+                                      PFlags & ~llvm::sys::Memory::MF_HUGE_HINT,
----------------
rnk wrote:
> This code pattern of a recursive tail call shows up in a few places in old LLVM code like this. Personally, I don't like it. It seems like a do / while loop around the mmap call itself would be clearer, since we don't need to re-run most of the flag calculations above. This would also avoid the Impl rename.
I agree that your suggestion is nicer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71975