[AIX][z/OS][Support] Provide no-op mapped_file_region::dontNeedImpl implementation

[daltenty]

Summary

mapped_file_region::dontNeedImpl added in D116366 calls madvise, which causes problems for z/OS and AIX.

For z/OS, we don't have either madvise, so treat this as a no-op, same as Windows does.

For AIX, it doesn't have any effect, doesn't have a standardized signature, and it needs certain feature test macros (i.e. _ALL_SOURCE) we don't set by default on for LLVM on AIX, so just make it a no-op too.

[aganea]

llvm/lib/Support/Unix/Path.inc

875–881

Shouldn't we protect the contents of this function with #ifdef HAVE_SYS_MMAN_H?

[daltenty]

Add HAVE_SYS_MMAN_H guard

[zibi]

llvm/lib/Support/Unix/Path.inc

875–881

I don't think we can use HAVE_SYS_MMAN_H since this is defined on z/OS since we do have sys/mman.h header file. We just don't have madvise().

[daltenty]

This comment was removed by daltenty.

[zibi]

LGTM, but I would prefer if we could use ::posix_madvise() not just on AIX. This will remove the unnecessary directive path.
However, we need to buy-in from the originator which introduced this call.

[daltenty]

llvm/lib/Support/Unix/Path.inc

875–881

The version here now should provide the fallback for both the case where we the header isn't available or if we are on z/OS, so I think we're ok.

[daltenty]

LGTM, but I would prefer if we could use ::posix_madvise() not just on AIX. This will remove the unnecessary directive path.

However, we need to buy-in from the originator which introduced this call.

Unfortunately it sounds like there is a different semantic for glibc on Linux though (namely the kernel is allowed to immediately discard dirty pages, which is why we assert the readonly property I assume), so the POSIX version is a no-op there, so we can't really unify the paths or we'll break the original behaviour. Worse, looking at the trace AIX I think we also don't make the system call, so I think we may need to abandon posix_madvise.

[daltenty]

Use madvise on AIX as well, since the semantics of posix_madvise don't match,

[daltenty]

Update comment

[daltenty]

Fix indent

[MaskRay]

POSIX specifies sys/mman.h and it is available on all major *NIX operation systems, so I don't think we need HAVE_SYS_MMAN_H.

We cannot use posix_madvise on Linux glibc/musl because MADV_DONTNEED (desired semantics on a read-only file mapping) is a no-op in their posix_madvise implementations.

If madvise doesn't have a benefit on AIX, consider just not implementing it to avoid some CMake complexity.

llvm/lib/Support/CMakeLists.txt

3

llvm/lib/Support/Unix/Path.inc

875

if (Mapping) { or use early return

[daltenty]

If madvise doesn't have a benefit on AIX, consider just not implementing it to avoid some CMake complexity.

I think you are right, I did some more digging and it seems like the OS is unlikely to use the information we are giving it any way:

The madvise subroutine is provided for compatibility only. The system takes no action on the advice specified.

https://www.ibm.com/docs/en/aix/7.1?topic=memory-understanding-mapping
https://www.ibm.com/docs/en/aix/7.1?topic=m-madvise-subroutine

I'll simplify this patch as suggested.

[daltenty]

Simply the patch since we've confirmed madvise doesn't have a useful effect on AIX anyway

[abhina.sreeskantharajan]

LGTM with minor formatting comment

llvm/lib/Support/Unix/Path.inc

874

Can this be #endif instead of #else? Not sure if this counts as not using an else after return.

[zibi]

LGTM

[daltenty]

llvm/lib/Support/Unix/Path.inc

874

IIUC what you're suggesting, we'd end up including the problem callsite bellow then, so I think we need to keep this.

llvm/lib/Support/CMakeLists.txt

 include(GetLibraryName)
+
+# Path requires definitions in headers which need _ALL_SOURCE to build on AIX
+if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")
+  remove_definitions("-D_XOPEN_SOURCE=700")
+  add_definitions("-D_ALL_SOURCE")
+endif()
+
 
 # Ensure that libSupport does not carry any static global initializer.
 # libSupport can be embedded in use cases where we don't want to load all
 # cl::opt unless we want to parse the command line.

llvm/lib/Support/Unix/Path.inc

 void mapped_file_region::dontNeedImpl() {
   assert(Mode == mapped_file_region::readonly);
   if (Mapping)
+#if defined(__MVS__) || !defined(HAVE_SYS_MMAN_H)
+    // If we don't have madvise, treat this as a no-op.
+    return;
+#elif defined(_AIX)
+    ::madvise((caddr_t)Mapping, Size, POSIX_MADV_DONTNEED);
+#else
     ::madvise(Mapping, Size, MADV_DONTNEED);
+#endif
 }
 
 int mapped_file_region::alignment() {