[libcxx] Recognize __UEFI__ predefine

[Prabhuk]

The new target triple x86_64-uefi does not set _WIN32 macro. It sets a
new predefine __UEFI__. Update platform.h to recognize if the target
object format is COFF based on __UEFI__ as well.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Prabhu Rajasekaran (Prabhuk)

Changes

The new target triple x86_64-uefi does not set _WIN32 macro. It sets a
new predefine UEFI. Update platform to recognize if the target
object format is COFF based on UEFI as well.

---

Full diff: https://github.com/llvm/llvm-project/pull/139327.diff

1 Files Affected:

• (modified) libcxx/include/__configuration/platform.h (+1-1)

diff --git a/libcxx/include/__configuration/platform.h b/libcxx/include/__configuration/platform.h
index f3c199dee172b..4a414d00e2208 100644
--- a/libcxx/include/__configuration/platform.h
+++ b/libcxx/include/__configuration/platform.h
@@ -20,7 +20,7 @@
 #  define _LIBCPP_OBJECT_FORMAT_ELF 1
 #elif defined(__MACH__)
 #  define _LIBCPP_OBJECT_FORMAT_MACHO 1
-#elif defined(_WIN32)
+#elif defined(_WIN32) || defined(__UEFI__)
 #  define _LIBCPP_OBJECT_FORMAT_COFF 1
 #elif defined(__wasm__)
 #  define _LIBCPP_OBJECT_FORMAT_WASM 1

[ldionne]

Is this introducing a new target? Are there going to be other changes for that target?

libc++ requires pre-commit CI to be set up for any newly supported target/platform, so I'd like to understand whether this falls into that category.

[Prabhuk]

Is this introducing a new target? Are there going to be other changes for that target?

libc++ requires pre-commit CI to be set up for any newly supported target/platform, so I'd like to understand whether this falls into that category.

I introduced a new target triple "x86_64-uefi" in Clang and been incrementally landing changes to support this triple. The __UEFI__ predefine is set when x86_64-uefi triple is used. This triple does not set _WIN32 and other windows specific predefines. At this point, this triple is in developmental stage. It will be good to have a pre-commit CI set up when the support is mature. I do not know if we are quite there yet at this point. Thank you.

[Prabhuk]

Is this introducing a new target? Are there going to be other changes for that target?
libc++ requires pre-commit CI to be set up for any newly supported target/platform, so I'd like to understand whether this falls into that category.

I introduced a new target triple "x86_64-uefi" in Clang and been incrementally landing changes to support this triple. The __UEFI__ predefine is set when x86_64-uefi triple is used. This triple does not set _WIN32 and other windows specific predefines. At this point, this triple is in developmental stage. It will be good to have a pre-commit CI set up when the support is mature. I do not know if we are quite there yet at this point. Thank you.

Adding a little bit more context. The way libcxx is used in the UEFI context I am experimenting with is header-only. We do not rely on the libcxx ABI and the changes I am expecting in the future will be limited to header files only. I would like to add precommit libcxx CI builders for UEFI when the support is mature but the builders will be limited to cover the header only scenarios which will presumably cover the changes I am expecting to land in libcxx.

[ldionne]

There is no such thing as using libc++ "header only". Even using basic functionality like std::sort requires libc++.so or libc++.a to be available, because we externally instantiate some specializations of std::sort into the built library. Failing to have a built library available will yield to linker errors when you do perfectly normal things like calling std::sort(int*, int*).

From your comment, this target is still experimental. I would rather add support in libc++ once you have a good understanding of the changes required to libc++ to support the target, and it looks like this might be a bit early for that. Otherwise, as far as the project is concerned, this patch is introducing dead code and our policy is to reject such patches.

[Prabhuk]

There is no such thing as using libc++ "header only". Even using basic functionality like std::sort requires libc++.so or libc++.a to be available, because we externally instantiate some specializations of std::sort into the built library. Failing to have a built library available will yield to linker errors when you do perfectly normal things like calling std::sort(int*, int*).

From your comment, this target is still experimental. I would rather add support in libc++ once you have a good understanding of the changes required to libc++ to support the target, and it looks like this might be a bit early for that. Otherwise, as far as the project is concerned, this patch is introducing dead code and our policy is to reject such patches.

That's fair. I'll look at what's the level of support we need from libcxx for UEFI and bring this back up when we get there. Thank you for your inputs.