[NFC] Address bit-field storage sizes to ensure ideal packing #139825

ojhunt · 2025-05-14T02:26:17Z

The MS bit-field packing ABI depends on the storage size of the type of being placed in the bit-field. This PR addresses a number of cases in llvm where the storage type has lead to suboptimal packing.

llvmbot · 2025-05-14T02:26:50Z

@llvm/pr-subscribers-llvm-support
@llvm/pr-subscribers-backend-aarch64
@llvm/pr-subscribers-debuginfo
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-backend-arm
@llvm/pr-subscribers-llvm-adt

@llvm/pr-subscribers-llvm-ir

Author: Oliver Hunt (ojhunt)

Changes

The MS bit-field packing ABI depends on the storage size of the type of being placed in the bit-field. This PR addresses a number of cases in llvm where the storage type has lead to suboptimal packing.

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

6 Files Affected:

(modified) llvm/include/llvm/ADT/ImmutableSet.h (+7-3)
(modified) llvm/include/llvm/Bitstream/BitCodes.h (+4-2)
(modified) llvm/include/llvm/Demangle/ItaniumDemangle.h (+6-4)
(modified) llvm/include/llvm/IR/Metadata.h (+2-2)
(modified) llvm/include/llvm/IR/ModuleSummaryIndex.h (+3-1)
(modified) llvm/include/llvm/IR/User.h (+2-2)

diff --git a/llvm/include/llvm/ADT/ImmutableSet.h b/llvm/include/llvm/ADT/ImmutableSet.h
index 5bee746688ce4..ac86f43b2048e 100644
--- a/llvm/include/llvm/ADT/ImmutableSet.h
+++ b/llvm/include/llvm/ADT/ImmutableSet.h
@@ -20,6 +20,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/iterator.h"
 #include "llvm/Support/Allocator.h"
+#include "llvm/Support/Compiler.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <cassert>
 #include <cstdint>
@@ -213,9 +214,12 @@ class ImutAVLTree {
   ImutAVLTree *next = nullptr;
 
   unsigned height : 28;
-  bool IsMutable : 1;
-  bool IsDigestCached : 1;
-  bool IsCanonicalized : 1;
+  LLVM_PREFERRED_TYPE(bool)
+  unsigned IsMutable : 1;
+  LLVM_PREFERRED_TYPE(bool)
+  unsigned IsDigestCached : 1;
+  LLVM_PREFERRED_TYPE(bool)
+  unsigned IsCanonicalized : 1;
 
   value_type value;
   uint32_t digest = 0;
diff --git a/llvm/include/llvm/Bitstream/BitCodes.h b/llvm/include/llvm/Bitstream/BitCodes.h
index 93888f7d3b335..3aff51de302dd 100644
--- a/llvm/include/llvm/Bitstream/BitCodes.h
+++ b/llvm/include/llvm/Bitstream/BitCodes.h
@@ -20,6 +20,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Bitstream/BitCodeEnums.h"
+#include "llvm/Support/Compiler.h"
 #include "llvm/Support/DataTypes.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <cassert>
@@ -32,8 +33,9 @@ namespace llvm {
 ///
 class BitCodeAbbrevOp {
   uint64_t Val;           // A literal value or data for an encoding.
-  bool IsLiteral : 1;     // Indicate whether this is a literal value or not.
-  unsigned Enc   : 3;     // The encoding to use.
+  LLVM_PREFERRED_TYPE(bool)
+  uint64_t IsLiteral : 1; // Indicate whether this is a literal value or not.
+  uint64_t Enc   : 3;     // The encoding to use.
 public:
   enum Encoding {
     Fixed = 1,  // A fixed width field, Val specifies number of bits.
diff --git a/llvm/include/llvm/Demangle/ItaniumDemangle.h b/llvm/include/llvm/Demangle/ItaniumDemangle.h
index 295c12ab24916..6abd0de787b13 100644
--- a/llvm/include/llvm/Demangle/ItaniumDemangle.h
+++ b/llvm/include/llvm/Demangle/ItaniumDemangle.h
@@ -19,6 +19,7 @@
 #include "DemangleConfig.h"
 #include "StringViewExtras.h"
 #include "Utility.h"
+#include "llvm/Support/Compiler.h"
 #include <algorithm>
 #include <cctype>
 #include <cstdio>
@@ -164,18 +165,18 @@ class NodeArray;
 // traversed by the printLeft/Right functions to produce a demangled string.
 class Node {
 public:
-  enum Kind : unsigned char {
+  enum Kind : unsigned {
 #define NODE(NodeKind) K##NodeKind,
 #include "ItaniumNodes.def"
   };
 
   /// Three-way bool to track a cached value. Unknown is possible if this node
   /// has an unexpanded parameter pack below it that may affect this cache.
-  enum class Cache : unsigned char { Yes, No, Unknown, };
+  enum class Cache : unsigned { Yes, No, Unknown, };
 
   /// Operator precedence for expression nodes. Used to determine required
   /// parens in expression emission.
-  enum class Prec {
+  enum class Prec : unsigned {
     Primary,
     Postfix,
     Unary,
@@ -2995,7 +2996,8 @@ template <typename Derived, typename Alloc> struct AbstractManglingParser {
     };
     char Enc[2];      // Encoding
     OIKind Kind;      // Kind of operator
-    bool Flag : 1;    // Entry-specific flag
+    LLVM_PREFERRED_TYPE(bool)
+    unsigned Flag : 1;    // Entry-specific flag
     Node::Prec Prec : 7; // Precedence
     const char *Name; // Spelling
 
diff --git a/llvm/include/llvm/IR/Metadata.h b/llvm/include/llvm/IR/Metadata.h
index 22ab59be55eb2..3d06edeed6c46 100644
--- a/llvm/include/llvm/IR/Metadata.h
+++ b/llvm/include/llvm/IR/Metadata.h
@@ -1076,8 +1076,8 @@ class MDNode : public Metadata {
   /// Explicity set alignment because bitfields by default have an
   /// alignment of 1 on z/OS.
   struct alignas(alignof(size_t)) Header {
-    bool IsResizable : 1;
-    bool IsLarge : 1;
+    size_t IsResizable : 1;
+    size_t IsLarge : 1;
     size_t SmallSize : 4;
     size_t SmallNumOps : 4;
     size_t : sizeof(size_t) * CHAR_BIT - 10;
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index 5080fa235905d..65e428a3adea7 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -28,6 +28,7 @@
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/Allocator.h"
+#include "llvm/Support/Compiler.h"
 #include "llvm/Support/InterleavedRange.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/ScaledNumber.h"
@@ -72,7 +73,8 @@ struct CalleeInfo {
   uint32_t Hotness : 3;
 
   // True if at least one of the calls to the callee is a tail call.
-  bool HasTailCall : 1;
+  LLVM_PREFERRED_TYPE(bool)
+  uint32_t HasTailCall : 1;
 
   /// The value stored in RelBlockFreq has to be interpreted as the digits of
   /// a scaled number with a scale of \p -ScaleShift.
diff --git a/llvm/include/llvm/IR/User.h b/llvm/include/llvm/IR/User.h
index 39e1314bd8130..be99e7a700374 100644
--- a/llvm/include/llvm/IR/User.h
+++ b/llvm/include/llvm/IR/User.h
@@ -79,8 +79,8 @@ class User : public Value {
   struct AllocInfo {
   public:
     const unsigned NumOps : NumUserOperandsBits;
-    const bool HasHungOffUses : 1;
-    const bool HasDescriptor : 1;
+    const unsigned HasHungOffUses : 1;
+    const unsigned HasDescriptor : 1;
 
     AllocInfo() = delete;

ojhunt · 2025-05-14T02:27:22Z

This is the first set of problems identified by -Wms-bitfield-padding I've attached @AaronBallman and @cor3ntin to this as the first set of issues identified by this new warning, but I would like llvm reviewers to comment.

github-actions · 2025-05-14T02:29:51Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff HEAD~1 HEAD --extensions h,cpp -- clang/include/clang/AST/DeclTemplate.h clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h clang/include/clang/Basic/DiagnosticCategories.h clang/include/clang/Sema/Overload.h clang/include/clang/Sema/ScopeInfo.h clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h clang/lib/Basic/DiagnosticIDs.cpp llvm/include/llvm/ADT/ImmutableSet.h llvm/include/llvm/Bitstream/BitCodes.h llvm/include/llvm/CodeGen/MachineInstr.h llvm/include/llvm/Demangle/ItaniumDemangle.h llvm/include/llvm/IR/Metadata.h llvm/include/llvm/IR/ModuleSummaryIndex.h llvm/include/llvm/IR/User.h llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h llvm/lib/Target/AArch64/AArch64CollectLOH.cpp llvm/lib/Target/ARM/ARMConstantIslandPass.cpp llvm/lib/Target/CSKY/CSKYConstantIslandPass.cpp llvm/lib/Target/Mips/MipsConstantIslandPass.cpp

View the diff from clang-format here.

diff --git a/clang/include/clang/Basic/DiagnosticCategories.h b/clang/include/clang/Basic/DiagnosticCategories.h
index 52bb7a268..f7b2af8d1 100644
--- a/clang/include/clang/Basic/DiagnosticCategories.h
+++ b/clang/include/clang/Basic/DiagnosticCategories.h
@@ -11,23 +11,23 @@
 
 namespace clang {
   namespace diag {
-    enum DiagCategory {
+  enum DiagCategory {
 #define GET_CATEGORY_TABLE
 #define CATEGORY(X, ENUM) ENUM,
 #include "clang/Basic/DiagnosticGroups.inc"
 #undef CATEGORY
 #undef GET_CATEGORY_TABLE
-      DiagCat_NUM_CATEGORIES
-    };
+    DiagCat_NUM_CATEGORIES
+  };
 
-    enum class Group {
-#define DIAG_ENTRY(GroupName, FlagNameOffset, Members, SubGroups, Docs)    \
-      GroupName,
+  enum class Group {
+#define DIAG_ENTRY(GroupName, FlagNameOffset, Members, SubGroups, Docs)      \
+    GroupName,
 #include "clang/Basic/DiagnosticGroups.inc"
 #undef CATEGORY
 #undef DIAG_ENTRY
-      NUM_GROUPS
-    };
+    NUM_GROUPS
+  };
   }  // end namespace diag
 }  // end namespace clang
 
diff --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
index 543ac8ab2..34c6621a2 100644
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -404,16 +404,14 @@ public:
   bool mayInlineCXXMemberFunction(CXXInlineableMemberKind K) const;
 
   ento::PathDiagnosticConsumerOptions getDiagOpts() const {
-    return {FullCompilerInvocation,
-            ShouldDisplayMacroExpansions,
+    return {FullCompilerInvocation, ShouldDisplayMacroExpansions,
             ShouldSerializeStats,
             // The stable report filename option is deprecated because
             // file names are now always stable. Now the old option acts as
             // an alias to the new verbose filename option because this
             // closely mimics the behavior under the old option.
             ShouldWriteStableReportFilename || ShouldWriteVerboseReportFilename,
-            static_cast<bool>(AnalyzerWerror),
-            ShouldApplyFixIts,
+            static_cast<bool>(AnalyzerWerror), ShouldApplyFixIts,
             ShouldDisplayCheckerNameForText};
   }
 };
diff --git a/llvm/include/llvm/Bitstream/BitCodes.h b/llvm/include/llvm/Bitstream/BitCodes.h
index 205024f75..3df106362 100644
--- a/llvm/include/llvm/Bitstream/BitCodes.h
+++ b/llvm/include/llvm/Bitstream/BitCodes.h
@@ -42,11 +42,11 @@ public:
   };
 
 protected:
-  uint64_t Val;           // A literal value or data for an encoding.
+  uint64_t Val; // A literal value or data for an encoding.
   LLVM_PREFERRED_TYPE(bool)
   uint64_t IsLiteral : 1; // Indicate whether this is a literal value or not.
   LLVM_PREFERRED_TYPE(Encoding)
-  uint64_t Enc : 3;       // The encoding to use.
+  uint64_t Enc : 3; // The encoding to use.
 
 public:
   static bool isValidEncoding(uint64_t E) {
@@ -100,7 +100,6 @@ public:
     return "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789._"
         [V];
   }
-
 };
 
 /// BitCodeAbbrev - This class represents an abbreviation record.  An
diff --git a/llvm/include/llvm/Demangle/ItaniumDemangle.h b/llvm/include/llvm/Demangle/ItaniumDemangle.h
index 7c24995e7..9e55bbfc7 100644
--- a/llvm/include/llvm/Demangle/ItaniumDemangle.h
+++ b/llvm/include/llvm/Demangle/ItaniumDemangle.h
@@ -172,7 +172,11 @@ public:
 
   /// Three-way bool to track a cached value. Unknown is possible if this node
   /// has an unexpanded parameter pack below it that may affect this cache.
-  enum class Cache : uint8_t { Yes, No, Unknown, };
+  enum class Cache : uint8_t {
+    Yes,
+    No,
+    Unknown,
+  };
 
   /// Operator precedence for expression nodes. Used to determine required
   /// parens in expression emission.
diff --git a/llvm/lib/Target/AArch64/AArch64CollectLOH.cpp b/llvm/lib/Target/AArch64/AArch64CollectLOH.cpp
index c3370cd6e..0bb449377 100644
--- a/llvm/lib/Target/AArch64/AArch64CollectLOH.cpp
+++ b/llvm/lib/Target/AArch64/AArch64CollectLOH.cpp
@@ -273,9 +273,9 @@ static int mapRegToGPRIndex(MCRegister Reg) {
 struct LOHInfo {
   MCLOHType Type : 8;           ///< "Best" type of LOH possible.
   LLVM_PREFERRED_TYPE(bool)
-  unsigned IsCandidate : 1;     ///< Possible LOH candidate.
+  unsigned IsCandidate : 1; ///< Possible LOH candidate.
   LLVM_PREFERRED_TYPE(bool)
-  unsigned OneUser : 1;         ///< Found exactly one user (yet).
+  unsigned OneUser : 1; ///< Found exactly one user (yet).
   LLVM_PREFERRED_TYPE(bool)
   unsigned MultiUsers : 1;      ///< Found multiple users.
   const MachineInstr *MI0;      ///< First instruction involved in the LOH.

The MS bit-field packing ABI depends on the storage size of the type of being placed in the bit-field. This PR addresses a number of cases in llvm where the storage type has lead to suboptimal packing.

ojhunt · 2025-05-14T02:37:30Z

(force push for formatting fixes as no one has reviewed this yet, one note: I intentionally did not update the Cache enum formatting as this is a non-structural change and I felt like this correction would not be warranted)

llvm/include/llvm/Bitstream/BitCodes.h

ojhunt · 2025-05-15T20:04:08Z

Sigh, my stage2 config didn't include -Wc++11-narrowing :-O

ojhunt · 2025-05-15T22:05:52Z

clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h

@@ -410,7 +412,7 @@ class AnalyzerOptions {
            // an alias to the new verbose filename option because this
            // closely mimics the behavior under the old option.
            ShouldWriteStableReportFilename || ShouldWriteVerboseReportFilename,
-            AnalyzerWerror,
+            static_cast<bool>(AnalyzerWerror),


This cast is needed before clang20 as we generate a warning about narrowing AnalyzerWerror from an int1 to a bool :D

ojhunt · 2025-05-15T22:08:45Z

clang/lib/Basic/DiagnosticIDs.cpp

  LLVM_PREFERRED_TYPE(bool)
-  uint8_t WarnShowInSystemMacro : 1;
+  uint16_t WarnShowInSystemMacro : 1;


I really wish we had some way of directly asserting the size/packing/offsets, imagine a hyopthetical

static_assert(__builtin_current_offset() == x)

It would make reasoning about layout significantly easier

ojhunt · 2025-05-15T23:03:22Z

clang/include/clang/Sema/Overload.h

@@ -996,7 +997,8 @@ class Sema;

    /// FailureKind - The reason why this candidate is not viable.
    /// Actually an OverloadFailureKind.


should I remove this comment now that there's a preferred type annotation?

ojhunt · 2025-05-15T23:03:56Z

clang/include/clang/Sema/ScopeInfo.h

@@ -103,7 +103,7 @@ enum class FirstCoroutineStmtKind { CoReturn, CoAwait, CoYield };
 /// currently being parsed.
 class FunctionScopeInfo {
 protected:
-  enum ScopeKind {
+  enum ScopeKind : uint8_t {


Given the restricted uses, making the storage type explicit here seemed like the best call

ojhunt · 2025-05-16T00:08:20Z

llvm/include/llvm/Support/MathExtras.h

@@ -40,6 +40,9 @@ template <typename T, typename U, typename = enableif_int<T, U>>
 using common_sint =
    std::common_type_t<std::make_signed_t<T>, std::make_signed_t<U>>;

+#pragma GCC diagnostic push


ignore this, it's an accidental inclusion I needed because I didn't set all the correct flags for the 2stage build

ojhunt · 2025-05-16T00:11:04Z

clang/include/clang/Basic/DiagnosticCategories.h

@@ -11,7 +11,7 @@

 namespace clang {
  namespace diag {
-    enum {
+    enum DiagCategory {


I figured naming this would not hurt, as I'm updating its storage and not specifying the type made me feel it was at risk of slipping through the cracks.

I've removed the storage sizes changes I was going to make so it could just be a correctly typed field as well on the basis that once I'm done with this round of changes I may go through and start trying to reduce the use of preferred type enums

ojhunt · 2025-05-16T00:11:46Z

clang/lib/Basic/DiagnosticIDs.cpp

  uint16_t OptionGroupIndex : 15;
+


What should be removed?

Would the size be reduced if DescriptionLen was a bit shorter?

@cor3ntin the extra line I added :D

Would the size be reduced if DescriptionLen was a bit shorter?

maybe? I was trying to make this not intentionally change/improve layout (on non-MS ABI), I figure that can be follow on work

ojhunt · 2025-05-16T00:20:51Z

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h

@@ -63,10 +63,10 @@ class LLVM_LIBRARY_VISIBILITY CodeViewDebug : public DebugHandlerBase {
    int DataOffset : 31;

    /// Non-zero if this is a piece of an aggregate.
-    uint16_t IsSubfield : 1;


This ideally would not be necessary as I believe this field should be 2 byte aligned here and so ms abi would not add unneeded padding, but when making this diagnostic I found that diagnosing this correctly required duplicating the record layout to essentially track the addition of otherwise unneeded padding, and I was concerned that you would get surprising behavior from otherwise "simple" changes (e.g. moving a bit-field around could suddenly unalign another field and trigger terrible padding)

…remove the MathExtras.h noise

ojhunt · 2025-05-16T06:59:30Z

The format problems are existing style that I don't want to reformat in this PR:

diff --git a/clang/include/clang/Basic/DiagnosticCategories.h b/clang/include/clang/Basic/DiagnosticCategories.h
index 52bb7a268b41..f7b2af8d194a 100644
--- a/clang/include/clang/Basic/DiagnosticCategories.h
+++ b/clang/include/clang/Basic/DiagnosticCategories.h
@@ -11,23 +11,23 @@
 
 namespace clang {
   namespace diag {
-    enum DiagCategory {
+  enum DiagCategory {
 #define GET_CATEGORY_TABLE
 #define CATEGORY(X, ENUM) ENUM,
 #include "clang/Basic/DiagnosticGroups.inc"
 #undef CATEGORY
 #undef GET_CATEGORY_TABLE
-      DiagCat_NUM_CATEGORIES
-    };
+    DiagCat_NUM_CATEGORIES
+  };
 
-    enum class Group {
-#define DIAG_ENTRY(GroupName, FlagNameOffset, Members, SubGroups, Docs)    \
-      GroupName,
+  enum class Group {
+#define DIAG_ENTRY(GroupName, FlagNameOffset, Members, SubGroups, Docs)      \
+    GroupName,
 #include "clang/Basic/DiagnosticGroups.inc"
 #undef CATEGORY
 #undef DIAG_ENTRY
-      NUM_GROUPS
-    };
+    NUM_GROUPS
+  };
   }  // end namespace diag
 }  // end namespace clang
 
diff --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
index 543ac8ab2ab1..34c6621a29c1 100644
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -404,16 +404,14 @@ public:
   bool mayInlineCXXMemberFunction(CXXInlineableMemberKind K) const;
 
   ento::PathDiagnosticConsumerOptions getDiagOpts() const {
-    return {FullCompilerInvocation,
-            ShouldDisplayMacroExpansions,
+    return {FullCompilerInvocation, ShouldDisplayMacroExpansions,
             ShouldSerializeStats,
             // The stable report filename option is deprecated because
             // file names are now always stable. Now the old option acts as
             // an alias to the new verbose filename option because this
             // closely mimics the behavior under the old option.
             ShouldWriteStableReportFilename || ShouldWriteVerboseReportFilename,
-            static_cast<bool>(AnalyzerWerror),
-            ShouldApplyFixIts,
+            static_cast<bool>(AnalyzerWerror), ShouldApplyFixIts,
             ShouldDisplayCheckerNameForText};
   }
 };
diff --git a/llvm/include/llvm/Bitstream/BitCodes.h b/llvm/include/llvm/Bitstream/BitCodes.h
index 205024f754df..3df10636203d 100644
--- a/llvm/include/llvm/Bitstream/BitCodes.h
+++ b/llvm/include/llvm/Bitstream/BitCodes.h
@@ -42,11 +42,11 @@ public:
   };
 
 protected:
-  uint64_t Val;           // A literal value or data for an encoding.
+  uint64_t Val; // A literal value or data for an encoding.
   LLVM_PREFERRED_TYPE(bool)
   uint64_t IsLiteral : 1; // Indicate whether this is a literal value or not.
   LLVM_PREFERRED_TYPE(Encoding)
-  uint64_t Enc : 3;       // The encoding to use.
+  uint64_t Enc : 3; // The encoding to use.
 
 public:
   static bool isValidEncoding(uint64_t E) {
@@ -100,7 +100,6 @@ public:
     return "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789._"
         [V];
   }
-
 };
 
 /// BitCodeAbbrev - This class represents an abbreviation record.  An
diff --git a/llvm/include/llvm/Demangle/ItaniumDemangle.h b/llvm/include/llvm/Demangle/ItaniumDemangle.h
index 7c24995e7463..9e55bbfc7d80 100644
--- a/llvm/include/llvm/Demangle/ItaniumDemangle.h
+++ b/llvm/include/llvm/Demangle/ItaniumDemangle.h
@@ -172,7 +172,11 @@ public:
 
   /// Three-way bool to track a cached value. Unknown is possible if this node
   /// has an unexpanded parameter pack below it that may affect this cache.
-  enum class Cache : uint8_t { Yes, No, Unknown, };
+  enum class Cache : uint8_t {
+    Yes,
+    No,
+    Unknown,
+  };
 
   /// Operator precedence for expression nodes. Used to determine required
   /// parens in expression emission.
diff --git a/llvm/lib/Target/AArch64/AArch64CollectLOH.cpp b/llvm/lib/Target/AArch64/AArch64CollectLOH.cpp
index c3370cd6e946..0bb4493773e1 100644
--- a/llvm/lib/Target/AArch64/AArch64CollectLOH.cpp
+++ b/llvm/lib/Target/AArch64/AArch64CollectLOH.cpp
@@ -273,9 +273,9 @@ static int mapRegToGPRIndex(MCRegister Reg) {
 struct LOHInfo {
   MCLOHType Type : 8;           ///< "Best" type of LOH possible.
   LLVM_PREFERRED_TYPE(bool)
-  unsigned IsCandidate : 1;     ///< Possible LOH candidate.
+  unsigned IsCandidate : 1; ///< Possible LOH candidate.
   LLVM_PREFERRED_TYPE(bool)
-  unsigned OneUser : 1;         ///< Found exactly one user (yet).
+  unsigned OneUser : 1; ///< Found exactly one user (yet).
   LLVM_PREFERRED_TYPE(bool)
   unsigned MultiUsers : 1;      ///< Found multiple users.
   const MachineInstr *MI0;      ///< First instruction involved in the LOH.

llvm-ci · 2025-05-16T07:10:20Z

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-win-fast running on as-builder-3 while building clang,llvm at step 6 "build-unified-tree".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/2/builds/24094

Here is the relevant piece of the build log for the reference

Step 6 (build-unified-tree) failure: build (failure)
...
[1165/4200] Building CXX object lib\CodeGen\SelectionDAG\CMakeFiles\LLVMSelectionDAG.dir\LegalizeVectorTypes.cpp.obj
[1166/4200] Building CXX object lib\CodeGen\AsmPrinter\CMakeFiles\LLVMAsmPrinter.dir\AsmPrinterDwarf.cpp.obj
[1167/4200] Building CXX object lib\DWARFLinker\CMakeFiles\LLVMDWARFLinker.dir\Utils.cpp.obj
[1168/4200] Building CXX object lib\CodeGen\AsmPrinter\CMakeFiles\LLVMAsmPrinter.dir\DIE.cpp.obj
[1169/4200] Building CXX object lib\DWARFLinker\CMakeFiles\LLVMDWARFLinker.dir\DWARFLinkerBase.cpp.obj
[1170/4200] Building CXX object lib\DWARFLinker\Classic\CMakeFiles\LLVMDWARFLinkerClassic.dir\DWARFLinkerDeclContext.cpp.obj
[1171/4200] Building CXX object lib\DWARFLinker\Classic\CMakeFiles\LLVMDWARFLinkerClassic.dir\DWARFLinkerCompileUnit.cpp.obj
[1172/4200] Building CXX object lib\CodeGen\AsmPrinter\CMakeFiles\LLVMAsmPrinter.dir\WinCFGuard.cpp.obj
[1173/4200] Building CXX object lib\CodeGen\GlobalISel\CMakeFiles\LLVMGlobalISel.dir\GISelChangeObserver.cpp.obj
[1174/4200] Building CXX object lib\CodeGen\AsmPrinter\CMakeFiles\LLVMAsmPrinter.dir\AsmPrinter.cpp.obj
FAILED: lib/CodeGen/AsmPrinter/CMakeFiles/LLVMAsmPrinter.dir/AsmPrinter.cpp.obj 
C:\ninja\ccache.exe C:\PROGRA~1\MICROS~2\2022\COMMUN~1\VC\Tools\MSVC\1438~1.331\bin\Hostx64\x64\cl.exe  /nologo /TP -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -IC:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\build\lib\CodeGen\AsmPrinter -IC:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\lib\CodeGen\AsmPrinter -IC:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\build\include -IC:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include /DWIN32 /D_WINDOWS   /Zc:inline /Zc:preprocessor /Zc:__cplusplus /Oi /bigobj /permissive- /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd5105 -wd4324 -wd4251 -wd4275 -w14062 -we4238 /Gw /O2 /Ob2 /DNDEBUG -MD  /EHs-c- /GR- -std:c++17 /showIncludes /Folib\CodeGen\AsmPrinter\CMakeFiles\LLVMAsmPrinter.dir\AsmPrinter.cpp.obj /Fdlib\CodeGen\AsmPrinter\CMakeFiles\LLVMAsmPrinter.dir\LLVMAsmPrinter.pdb /FS -c C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\lib\CodeGen\AsmPrinter\AsmPrinter.cpp
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\lib\CodeGen\AsmPrinter\CodeViewDebug.h(88): error C2607: static assertion failed
[1175/4200] Building CXX object lib\CodeGen\AsmPrinter\CMakeFiles\LLVMAsmPrinter.dir\AccelTable.cpp.obj
[1176/4200] Building CXX object lib\CodeGen\AsmPrinter\CMakeFiles\LLVMAsmPrinter.dir\WasmException.cpp.obj
[1177/4200] Building CXX object lib\CodeGen\SelectionDAG\CMakeFiles\LLVMSelectionDAG.dir\SelectionDAGDumper.cpp.obj
[1178/4200] Building CXX object lib\CodeGen\AsmPrinter\CMakeFiles\LLVMAsmPrinter.dir\DwarfExpression.cpp.obj
[1179/4200] Building CXX object lib\CodeGen\AsmPrinter\CMakeFiles\LLVMAsmPrinter.dir\DwarfFile.cpp.obj
[1180/4200] Building CXX object lib\CodeGen\AsmPrinter\CMakeFiles\LLVMAsmPrinter.dir\OcamlGCPrinter.cpp.obj
[1181/4200] Building CXX object lib\CodeGen\AsmPrinter\CMakeFiles\LLVMAsmPrinter.dir\EHStreamer.cpp.obj
[1182/4200] Building CXX object lib\CodeGen\GlobalISel\CMakeFiles\LLVMGlobalISel.dir\LegalityPredicates.cpp.obj
[1183/4200] Building CXX object lib\CodeGen\SelectionDAG\CMakeFiles\LLVMSelectionDAG.dir\LegalizeDAG.cpp.obj
[1184/4200] Building CXX object lib\CodeGen\GlobalISel\CMakeFiles\LLVMGlobalISel.dir\LegalizeMutations.cpp.obj
[1185/4200] Building CXX object lib\CodeGen\GlobalISel\CMakeFiles\LLVMGlobalISel.dir\LegalizerInfo.cpp.obj
[1186/4200] Building CXX object lib\CodeGen\GlobalISel\CMakeFiles\LLVMGlobalISel.dir\InstructionSelector.cpp.obj
[1187/4200] Building CXX object lib\CodeGen\GlobalISel\CMakeFiles\LLVMGlobalISel.dir\GISelValueTracking.cpp.obj
[1188/4200] Building CXX object lib\CodeGen\AsmPrinter\CMakeFiles\LLVMAsmPrinter.dir\DbgEntityHistoryCalculator.cpp.obj
[1189/4200] Building CXX object lib\CodeGen\GlobalISel\CMakeFiles\LLVMGlobalISel.dir\CSEInfo.cpp.obj
[1190/4200] Building CXX object lib\CodeGen\AsmPrinter\CMakeFiles\LLVMAsmPrinter.dir\DIEHash.cpp.obj
[1191/4200] Building CXX object lib\CodeGen\GlobalISel\CMakeFiles\LLVMGlobalISel.dir\LegacyLegalizerInfo.cpp.obj
[1192/4200] Building CXX object lib\CodeGen\AsmPrinter\CMakeFiles\LLVMAsmPrinter.dir\ErlangGCPrinter.cpp.obj
[1193/4200] Building CXX object lib\CodeGen\CMakeFiles\LLVMCodeGen.dir\LiveDebugValues\InstrRefBasedImpl.cpp.obj
[1194/4200] Building CXX object lib\CodeGen\AsmPrinter\CMakeFiles\LLVMAsmPrinter.dir\WinException.cpp.obj
[1195/4200] Building CXX object lib\CodeGen\GlobalISel\CMakeFiles\LLVMGlobalISel.dir\CSEMIRBuilder.cpp.obj
[1196/4200] Building CXX object lib\CodeGen\GlobalISel\CMakeFiles\LLVMGlobalISel.dir\CombinerHelperCompares.cpp.obj
[1197/4200] Building CXX object lib\CodeGen\AsmPrinter\CMakeFiles\LLVMAsmPrinter.dir\PseudoProbePrinter.cpp.obj
[1198/4200] Building CXX object lib\Frontend\Driver\CMakeFiles\LLVMFrontendDriver.dir\CodeGenOptions.cpp.obj
[1199/4200] Building CXX object lib\CodeGen\GlobalISel\CMakeFiles\LLVMGlobalISel.dir\LostDebugLocObserver.cpp.obj
[1200/4200] Building CXX object lib\Bitcode\Reader\CMakeFiles\LLVMBitReader.dir\BitcodeAnalyzer.cpp.obj
[1201/4200] Building CXX object lib\Bitcode\Reader\CMakeFiles\LLVMBitReader.dir\BitReader.cpp.obj
[1202/4200] Building CXX object lib\CodeGen\SelectionDAG\CMakeFiles\LLVMSelectionDAG.dir\SelectionDAGISel.cpp.obj
[1203/4200] Building CXX object lib\CodeGen\GlobalISel\CMakeFiles\LLVMGlobalISel.dir\CallLowering.cpp.obj
[1204/4200] Building CXX object lib\Bitcode\Writer\CMakeFiles\LLVMBitWriter.dir\BitcodeWriterPass.cpp.obj
[1205/4200] Building CXX object lib\CodeGen\GlobalISel\CMakeFiles\LLVMGlobalISel.dir\GIMatchTableExecutor.cpp.obj
[1206/4200] Building CXX object lib\Bitcode\Reader\CMakeFiles\LLVMBitReader.dir\ValueList.cpp.obj
[1207/4200] Building CXX object lib\CodeGen\GlobalISel\CMakeFiles\LLVMGlobalISel.dir\InstructionSelect.cpp.obj
[1208/4200] Building CXX object lib\Bitcode\Writer\CMakeFiles\LLVMBitWriter.dir\BitWriter.cpp.obj
[1209/4200] Building CXX object lib\CodeGen\GlobalISel\CMakeFiles\LLVMGlobalISel.dir\Combiner.cpp.obj
[1210/4200] Building CXX object lib\CodeGen\AsmPrinter\CMakeFiles\LLVMAsmPrinter.dir\CodeViewDebug.cpp.obj

ojhunt · 2025-05-16T07:49:41Z

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-win-fast running on as-builder-3 while building clang,llvm at step 6 "build-unified-tree".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/2/builds/24094

Here is the relevant piece of the build log for the reference

sigh, fixed with https://github.com/llvm/llvm-project/pull/140214/files

ItaniumDemangle.h exists in both llvm/ and libcxxabi/. These files are supposed to be copies of each other (minus the top two lines). This patch adds a test to assert that this is the case to enable tooling to automatically detect this as an issue, like in llvm#139825. This makes it easier for contributors unfamiliar with the duplication to make changes/get appropriate reviews. Ideally we would share the file and copy it from one place to the other but the ideal way to do this (based on previous discussion with libc++ maintainers) would be a new runtime library that clearly outlines requirements, so that is left for later with the test being used as a stopgap. This is a relatively common approach for structures shared between compiler-rt and LLVM. This patch does make the test reference the LLVM source directory, but that should be fine given building libcxxabi is only supported through the runtimes build in the monorepo meaning it should always be available.

Follow on work from #139825

Follow on work from #139825. As previously this is fixing a few more cases where adjacent bit-fields have types with different base storage sizes, as the MS ABI will not pack those bit-fields.

…40323) ItaniumDemangle.h exists in both llvm/ and libcxxabi/. These files are supposed to be copies of each other (minus the top two lines). This patch adds a test to assert that this is the case to enable tooling to automatically detect this as an issue, like in #139825. This makes it easier for contributors unfamiliar with the duplication to make changes/get appropriate reviews. Ideally we would share the file and copy it from one place to the other but the ideal way to do this (based on previous discussion with libc++ maintainers) would be a new runtime library that clearly outlines requirements, so that is left for later with the test being used as a stopgap. This is a relatively common approach for structures shared between compiler-rt and LLVM. This patch does make the test reference the LLVM source directory, but that should be fine given building libcxxabi is only supported through the runtimes build in the monorepo meaning it should always be available.

…vm#140323) ItaniumDemangle.h exists in both llvm/ and libcxxabi/. These files are supposed to be copies of each other (minus the top two lines). This patch adds a test to assert that this is the case to enable tooling to automatically detect this as an issue, like in llvm#139825. This makes it easier for contributors unfamiliar with the duplication to make changes/get appropriate reviews. Ideally we would share the file and copy it from one place to the other but the ideal way to do this (based on previous discussion with libc++ maintainers) would be a new runtime library that clearly outlines requirements, so that is left for later with the test being used as a stopgap. This is a relatively common approach for structures shared between compiler-rt and LLVM. This patch does make the test reference the LLVM source directory, but that should be fine given building libcxxabi is only supported through the runtimes build in the monorepo meaning it should always be available.

ojhunt requested review from cor3ntin, AaronBallman, dpaoliello and teresajohnson May 14, 2025 02:26

llvmbot added llvm:ir llvm:adt labels May 14, 2025

ojhunt self-assigned this May 14, 2025

[NFC] Address bit-field storage sizes to ensure ideal packing

a28452d

The MS bit-field packing ABI depends on the storage size of the type of being placed in the bit-field. This PR addresses a number of cases in llvm where the storage type has lead to suboptimal packing.

ojhunt force-pushed the users/ojhunt/bit-field-storage-fixes branch from a5e84aa to a28452d Compare May 14, 2025 02:34

ojhunt mentioned this pull request May 14, 2025

Add -Wms-bitfield-padding warning when possible #139828

Draft

AaronBallman reviewed May 14, 2025

View reviewed changes

llvm/include/llvm/Bitstream/BitCodes.h Outdated Show resolved Hide resolved

AaronBallman requested a review from nikic May 14, 2025 11:57

More storage fixes

3a2b2f2

llvmbot added clang Clang issues not falling into any other category backend:ARM backend:AArch64 clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:static analyzer debuginfo backend:CSKY llvm:support clang:analysis labels May 15, 2025

Address erroneous warning in clang <= 19.0

868f0a9

ojhunt commented May 15, 2025

View reviewed changes

More changes to make things better, for a given value of better

980a6fa

ojhunt commented May 16, 2025

View reviewed changes

add preferred type annotations i inexplicably didn't add earlier and …

c74da90

…remove the MathExtras.h noise

cor3ntin approved these changes May 16, 2025

View reviewed changes

remove excess newline

004ff93

ojhunt merged commit 76ba29b into main May 16, 2025
5 of 10 checks passed

ojhunt deleted the users/ojhunt/bit-field-storage-fixes branch May 16, 2025 07:03

boomanaiden154 mentioned this pull request May 17, 2025

[libcxxabi] Add test to assert that ItaniumDemangle.h is the same #140323

Merged

ojhunt added a commit that referenced this pull request May 19, 2025

[NFC] Address more bit-field storage sizes

6234ea2

Follow on work from #139825

ojhunt mentioned this pull request May 19, 2025

[NFC] Address more bit-field storage sizes #140493

Open

ojhunt added a commit that referenced this pull request May 19, 2025

[NFC] Address more bit-field storage sizes

90e4c9d

Follow on work from #139825

		@@ -996,7 +997,8 @@ class Sema;

		/// FailureKind - The reason why this candidate is not viable.
		/// Actually an OverloadFailureKind.

[NFC] Address bit-field storage sizes to ensure ideal packing #139825

[NFC] Address bit-field storage sizes to ensure ideal packing #139825

Uh oh!

Conversation

ojhunt commented May 14, 2025

Uh oh!

llvmbot commented May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ojhunt commented May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ojhunt commented May 14, 2025

Uh oh!

Uh oh!

ojhunt commented May 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ojhunt May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ojhunt commented May 16, 2025

Uh oh!

Uh oh!

llvm-ci commented May 16, 2025

Uh oh!

ojhunt commented May 16, 2025

Uh oh!

Uh oh!

llvmbot commented May 14, 2025 •

edited

Loading

ojhunt commented May 14, 2025 •

edited

Loading

github-actions bot commented May 14, 2025 •

edited

Loading

ojhunt May 16, 2025 •

edited

Loading