[lld-macho] Disable static assert failing on several platforms

[alx32]

Recently #106573 modified Define and caused the size to change on some platforms.
Issue: #107511

Disable the static assert it until we can find a universal solution.

[llvmbot]

@llvm/pr-subscribers-lld

Author: None (alx32)

Changes

Recently #106573 modified Define and caused the size to change on some platforms.
#107511

Disable the static assert it until we can find a universal solution.

---

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

1 Files Affected:

• (modified) lld/MachO/Symbols.cpp (+4)

diff --git a/lld/MachO/Symbols.cpp b/lld/MachO/Symbols.cpp
index 9faf01e09de059..61becf97ba6728 100644
--- a/lld/MachO/Symbols.cpp
+++ b/lld/MachO/Symbols.cpp
@@ -18,12 +18,16 @@ using namespace lld::macho;
 static_assert(sizeof(void *) != 8 || sizeof(Symbol) == 56,
               "Try to minimize Symbol's size; we create many instances");
 
+// TODO: Fix this for all platforms or remove it.
+// https://github.com/llvm/llvm-project/issues/107511
+#if 0
 // The Microsoft ABI doesn't support using parent class tail padding for child
 // members, hence the _MSC_VER check.
 #if !defined(_MSC_VER)
 static_assert(sizeof(void *) != 8 || sizeof(Defined) == 88,
               "Try to minimize Defined's size; we create many instances");
 #endif
+#endif
 
 static_assert(sizeof(SymbolUnion) == sizeof(Defined),
               "Defined should be the largest Symbol kind");

[llvmbot]

@llvm/pr-subscribers-lld-macho

Author: None (alx32)

Changes

Recently #106573 modified Define and caused the size to change on some platforms.
#107511

Disable the static assert it until we can find a universal solution.

---

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

1 Files Affected:

• (modified) lld/MachO/Symbols.cpp (+4)

diff --git a/lld/MachO/Symbols.cpp b/lld/MachO/Symbols.cpp
index 9faf01e09de059..61becf97ba6728 100644
--- a/lld/MachO/Symbols.cpp
+++ b/lld/MachO/Symbols.cpp
@@ -18,12 +18,16 @@ using namespace lld::macho;
 static_assert(sizeof(void *) != 8 || sizeof(Symbol) == 56,
               "Try to minimize Symbol's size; we create many instances");
 
+// TODO: Fix this for all platforms or remove it.
+// https://github.com/llvm/llvm-project/issues/107511
+#if 0
 // The Microsoft ABI doesn't support using parent class tail padding for child
 // members, hence the _MSC_VER check.
 #if !defined(_MSC_VER)
 static_assert(sizeof(void *) != 8 || sizeof(Defined) == 88,
               "Try to minimize Defined's size; we create many instances");
 #endif
+#endif
 
 static_assert(sizeof(SymbolUnion) == sizeof(Defined),
               "Defined should be the largest Symbol kind");

[mstorsjo]

LGTM, this would fix the build for mingw targets.

[BertalanD]

I think the issue is that Windows targets only "merge" bit fields when the underlying types have the same size. Can someone check that this fixes the issue?

diff --git a/lld/MachO/Symbols.h b/lld/MachO/Symbols.h
index 70fd195f25c9..bfb777b86621 100644
--- a/lld/MachO/Symbols.h
+++ b/lld/MachO/Symbols.h
@@ -14,6 +14,7 @@
 #include "Target.h"
 
 #include "llvm/Object/Archive.h"
+#include "llvm/Support/Compiler.h"
 #include "llvm/Support/MathExtras.h"
 
 namespace lld {
@@ -152,7 +153,8 @@ public:
   // Whether this symbol should appear in the output symbol table.
   bool includeInSymtab : 1;
   // The ICF folding kind of this symbol: None / Body / Thunk.
-  ICFFoldKind identicalCodeFoldingKind : 2;
+  LLVM_PREFERRED_TYPE(ICFFoldKind)
+  uint8_t identicalCodeFoldingKind : 2;
   // Symbols marked referencedDynamically won't be removed from the output's
   // symbol table by tools like strip. In theory, this could be set on arbitrary
   // symbols in input object files. In practice, it's used solely for the

[mstorsjo]

I think the issue is that Windows targets only "merge" bit fields when the underlying types have the same size. Can someone check that this fixes the issue?

diff --git a/lld/MachO/Symbols.h b/lld/MachO/Symbols.h
index 70fd195f25c9..bfb777b86621 100644
--- a/lld/MachO/Symbols.h
+++ b/lld/MachO/Symbols.h
@@ -14,6 +14,7 @@
 #include "Target.h"
 
 #include "llvm/Object/Archive.h"
+#include "llvm/Support/Compiler.h"
 #include "llvm/Support/MathExtras.h"
 
 namespace lld {
@@ -152,7 +153,8 @@ public:
   // Whether this symbol should appear in the output symbol table.
   bool includeInSymtab : 1;
   // The ICF folding kind of this symbol: None / Body / Thunk.
-  ICFFoldKind identicalCodeFoldingKind : 2;
+  LLVM_PREFERRED_TYPE(ICFFoldKind)
+  uint8_t identicalCodeFoldingKind : 2;
   // Symbols marked referencedDynamically won't be removed from the output's
   // symbol table by tools like strip. In theory, this could be set on arbitrary
   // symbols in input object files. In practice, it's used solely for the

Yes, this does seem to fix the issue for me, too. Thanks!

[BertalanD]

Alternative fix: #107545

[mstorsjo]

With the alternative fix merged already, this PR shouldn't be necessary any longer.