[libc][msvc] fix mathlib build on WoA

[SchrodingerZhu]

Fix build errors encountered when building math library on WoA.

1. Skip FEnv equality check for MSVC
2. Provide a placeholder type for vector types.

[llvmbot]

@llvm/pr-subscribers-libc

Author: Schrodinger ZHU Yifan (SchrodingerZhu)

Changes

---

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

2 Files Affected:

• (modified) libc/src/string/memory_utils/op_generic.h (+14-3)
• (modified) libc/test/UnitTest/FEnvSafeTest.cpp (+1-1)

diff --git a/libc/src/string/memory_utils/op_generic.h b/libc/src/string/memory_utils/op_generic.h
index 010f2187a4ffd..683c9e1cd6925 100644
--- a/libc/src/string/memory_utils/op_generic.h
+++ b/libc/src/string/memory_utils/op_generic.h
@@ -40,13 +40,23 @@ static_assert((UINTPTR_MAX == 4294967295U) ||
                   (UINTPTR_MAX == 18446744073709551615UL),
               "We currently only support 32- or 64-bit platforms");
 
-#ifdef LIBC_COMPILER_IS_MSVC
-
+#ifdef LIBC_COMPILER_IS_MSVC 
+#ifdef LIBC_TARGET_ARCH_IS_X86
 namespace LIBC_NAMESPACE_DECL {
 using generic_v128 = __m128i;
 using generic_v256 = __m256i;
 using generic_v512 = __m512i;
 } // namespace LIBC_NAMESPACE_DECL
+#else
+// Special handling when target does not have real vector types.
+// We can potentially use uint8x16_t etc. However, MSVC does not provide
+// subscript operation.
+namespace LIBC_NAMESPACE_DECL {
+struct alignas(16) generic_v128 : public cpp::array<uint8_t, 16> {};
+struct alignas(32) generic_v256 : public cpp::array<uint8_t, 32> {};
+struct alignas(64) generic_v512 : public cpp::array<uint8_t, 64> {};
+}
+#endif
 
 #else
 namespace LIBC_NAMESPACE_DECL {
@@ -159,7 +169,8 @@ template <typename T> struct Memset {
 
   LIBC_INLINE static void block(Ptr dst, uint8_t value) {
     if constexpr (is_scalar_v<T> || is_vector_v<T>) {
-      store<T>(dst, splat<T>(value));
+      // Avoid ambiguous call due to ADT
+      generic::store<T>(dst, splat<T>(value));
     } else if constexpr (is_array_v<T>) {
       using value_type = typename T::value_type;
       const auto Splat = splat<value_type>(value);
diff --git a/libc/test/UnitTest/FEnvSafeTest.cpp b/libc/test/UnitTest/FEnvSafeTest.cpp
index 2730de350b39a..4393f9d5e5c3b 100644
--- a/libc/test/UnitTest/FEnvSafeTest.cpp
+++ b/libc/test/UnitTest/FEnvSafeTest.cpp
@@ -43,7 +43,7 @@ void FEnvSafeTest::set_fenv(const fenv_t &fenv) {
 
 void FEnvSafeTest::expect_fenv_eq(const fenv_t &before_fenv,
                                   const fenv_t &after_fenv) {
-#if defined(LIBC_TARGET_ARCH_IS_AARCH64)
+#if defined(LIBC_TARGET_ARCH_IS_AARCH64) && !defined(LIBC_COMPILER_IS_MSVC)
   using FPState = LIBC_NAMESPACE::fputil::FEnv::FPState;
   const FPState &before_state = reinterpret_cast<const FPState &>(before_fenv);
   const FPState &after_state = reinterpret_cast<const FPState &>(after_fenv);

[Copilot]

Pull Request Overview

This PR fixes build errors encountered when building the math library on Windows on ARM (WoA) with MSVC compiler by addressing two specific compilation issues.

• Skip FEnv equality check for MSVC on ARM64 architecture
• Add placeholder vector types for non-x86 architectures when using MSVC compiler
• Resolve ambiguous function call by explicitly qualifying the store function

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
libc/test/UnitTest/FEnvSafeTest.cpp	Excludes MSVC from ARM64-specific FEnv equality check
libc/src/string/memory_utils/op_generic.h	Adds placeholder vector types for MSVC non-x86 and fixes ambiguous function call

[Copilot]

The placeholder vector type structs lack documentation explaining their purpose as MSVC-specific workarounds. Add comments explaining why these are needed and when they're used instead of native vector types.

Suggested change

	namespace LIBC_NAMESPACE_DECL {
	namespace LIBC_NAMESPACE_DECL {
	// MSVC-specific workaround:
	// On non-x86 targets, MSVC does not provide native vector types or subscriptable vector types
	// (such as uint8x16_t). As a workaround, we define these placeholder structs as aligned arrays
	// of uint8_t. These are used in place of native vector types when compiling with MSVC on
	// targets that lack real vector support.

[Copilot]

The comment mentions ADL (Argument-Dependent Lookup) but doesn't explain what specific ambiguity is being resolved. Consider expanding the comment to clarify which functions were conflicting and why the generic:: qualification is necessary.

Suggested change

	// Avoid ambiguous call due to ADL
	// Avoid ambiguous call due to ADL: There are multiple 'store' functions in different namespaces,
	// including a global 'store' and 'generic::store'. Without the 'generic::' qualification,
	// the compiler may be unable to resolve which 'store' to use due to argument-dependent lookup (ADL),
	// leading to ambiguity. Explicitly qualifying with 'generic::' ensures the correct function is called.

[SchrodingerZhu]

@jhuber6 enforcing the existence of generic_v512 looks suspicious. Do you know if this is really required?