From 32f1c1d85e6ef9b0fa32438e64d5f319ee498ff9 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 7 Dec 2024 22:27:23 +1100 Subject: [PATCH 1/2] Make our `DIFlags` match `LLVMDIFlags` in the LLVM-C API --- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 20 +- .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 182 +++++++----------- 2 files changed, 85 insertions(+), 117 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index ec6c84f6f2567..4062da767c1f9 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -741,8 +741,11 @@ pub mod debuginfo { pub type DIEnumerator = DIDescriptor; pub type DITemplateTypeParameter = DIDescriptor; - // These values **must** match with LLVMRustDIFlags!! bitflags! { + /// Must match the layout of `LLVMDIFlags` in the LLVM-C API. + /// + /// Each value declared here must also be covered by the static + /// assertions in `RustWrapper.cpp` used by `fromRust(LLVMDIFlags)`. #[repr(transparent)] #[derive(Clone, Copy, Default)] pub struct DIFlags: u32 { @@ -752,7 +755,7 @@ pub mod debuginfo { const FlagPublic = 3; const FlagFwdDecl = (1 << 2); const FlagAppleBlock = (1 << 3); - const FlagBlockByrefStruct = (1 << 4); + const FlagReservedBit4 = (1 << 4); const FlagVirtual = (1 << 5); const FlagArtificial = (1 << 6); const FlagExplicit = (1 << 7); @@ -763,10 +766,21 @@ pub mod debuginfo { const FlagStaticMember = (1 << 12); const FlagLValueReference = (1 << 13); const FlagRValueReference = (1 << 14); - const FlagExternalTypeRef = (1 << 15); + const FlagReserved = (1 << 15); + const FlagSingleInheritance = (1 << 16); + const FlagMultipleInheritance = (2 << 16); + const FlagVirtualInheritance = (3 << 16); const FlagIntroducedVirtual = (1 << 18); const FlagBitField = (1 << 19); const FlagNoReturn = (1 << 20); + // The bit at (1 << 21) is unused, but was `LLVMDIFlagMainSubprogram`. + const FlagTypePassByValue = (1 << 22); + const FlagTypePassByReference = (1 << 23); + const FlagEnumClass = (1 << 24); + const FlagThunk = (1 << 25); + const FlagNonTrivial = (1 << 26); + const FlagBigEndian = (1 << 27); + const FlagLittleEndian = (1 << 28); } } diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index dd72ea2497f7c..35186778671bc 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -2,6 +2,7 @@ #include "llvm-c/Analysis.h" #include "llvm-c/Core.h" +#include "llvm-c/DebugInfo.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/Statistic.h" @@ -676,120 +677,73 @@ template DIT *unwrapDIPtr(LLVMMetadataRef Ref) { #define DIArray DINodeArray #define unwrapDI unwrapDIPtr -// These values **must** match debuginfo::DIFlags! They also *happen* -// to match LLVM, but that isn't required as we do giant sets of -// matching below. The value shouldn't be directly passed to LLVM. -enum class LLVMRustDIFlags : uint32_t { - FlagZero = 0, - FlagPrivate = 1, - FlagProtected = 2, - FlagPublic = 3, - FlagFwdDecl = (1 << 2), - FlagAppleBlock = (1 << 3), - FlagBlockByrefStruct = (1 << 4), - FlagVirtual = (1 << 5), - FlagArtificial = (1 << 6), - FlagExplicit = (1 << 7), - FlagPrototyped = (1 << 8), - FlagObjcClassComplete = (1 << 9), - FlagObjectPointer = (1 << 10), - FlagVector = (1 << 11), - FlagStaticMember = (1 << 12), - FlagLValueReference = (1 << 13), - FlagRValueReference = (1 << 14), - FlagExternalTypeRef = (1 << 15), - FlagIntroducedVirtual = (1 << 18), - FlagBitField = (1 << 19), - FlagNoReturn = (1 << 20), - // Do not add values that are not supported by the minimum LLVM - // version we support! see llvm/include/llvm/IR/DebugInfoFlags.def -}; - -inline LLVMRustDIFlags operator&(LLVMRustDIFlags A, LLVMRustDIFlags B) { - return static_cast(static_cast(A) & - static_cast(B)); -} - -inline LLVMRustDIFlags operator|(LLVMRustDIFlags A, LLVMRustDIFlags B) { - return static_cast(static_cast(A) | - static_cast(B)); -} - -inline LLVMRustDIFlags &operator|=(LLVMRustDIFlags &A, LLVMRustDIFlags B) { - return A = A | B; -} - -inline bool isSet(LLVMRustDIFlags F) { return F != LLVMRustDIFlags::FlagZero; } - -inline LLVMRustDIFlags visibility(LLVMRustDIFlags F) { - return static_cast(static_cast(F) & 0x3); -} - -static DINode::DIFlags fromRust(LLVMRustDIFlags Flags) { - DINode::DIFlags Result = DINode::DIFlags::FlagZero; - - switch (visibility(Flags)) { - case LLVMRustDIFlags::FlagPrivate: - Result |= DINode::DIFlags::FlagPrivate; - break; - case LLVMRustDIFlags::FlagProtected: - Result |= DINode::DIFlags::FlagProtected; - break; - case LLVMRustDIFlags::FlagPublic: - Result |= DINode::DIFlags::FlagPublic; - break; - default: - // The rest are handled below - break; - } - - if (isSet(Flags & LLVMRustDIFlags::FlagFwdDecl)) { - Result |= DINode::DIFlags::FlagFwdDecl; - } - if (isSet(Flags & LLVMRustDIFlags::FlagAppleBlock)) { - Result |= DINode::DIFlags::FlagAppleBlock; - } - if (isSet(Flags & LLVMRustDIFlags::FlagVirtual)) { - Result |= DINode::DIFlags::FlagVirtual; - } - if (isSet(Flags & LLVMRustDIFlags::FlagArtificial)) { - Result |= DINode::DIFlags::FlagArtificial; - } - if (isSet(Flags & LLVMRustDIFlags::FlagExplicit)) { - Result |= DINode::DIFlags::FlagExplicit; - } - if (isSet(Flags & LLVMRustDIFlags::FlagPrototyped)) { - Result |= DINode::DIFlags::FlagPrototyped; - } - if (isSet(Flags & LLVMRustDIFlags::FlagObjcClassComplete)) { - Result |= DINode::DIFlags::FlagObjcClassComplete; - } - if (isSet(Flags & LLVMRustDIFlags::FlagObjectPointer)) { - Result |= DINode::DIFlags::FlagObjectPointer; - } - if (isSet(Flags & LLVMRustDIFlags::FlagVector)) { - Result |= DINode::DIFlags::FlagVector; - } - if (isSet(Flags & LLVMRustDIFlags::FlagStaticMember)) { - Result |= DINode::DIFlags::FlagStaticMember; - } - if (isSet(Flags & LLVMRustDIFlags::FlagLValueReference)) { - Result |= DINode::DIFlags::FlagLValueReference; - } - if (isSet(Flags & LLVMRustDIFlags::FlagRValueReference)) { - Result |= DINode::DIFlags::FlagRValueReference; - } - if (isSet(Flags & LLVMRustDIFlags::FlagIntroducedVirtual)) { - Result |= DINode::DIFlags::FlagIntroducedVirtual; - } - if (isSet(Flags & LLVMRustDIFlags::FlagBitField)) { - Result |= DINode::DIFlags::FlagBitField; - } - if (isSet(Flags & LLVMRustDIFlags::FlagNoReturn)) { - Result |= DINode::DIFlags::FlagNoReturn; - } - - return Result; +// FIXME(Zalathar): This is a temporary typedef to avoid churning dozens of +// bindings that are going to be deleted and replaced with their LLVM-C +// equivalents, as part of #134009. After that happens, the remaining bindings +// can be adjusted to use `LLVMDIFlags` instead of relying on this typedef. +typedef LLVMDIFlags LLVMRustDIFlags; + +// Statically assert that `LLVMDIFlags` (C) and `DIFlags` (C++) have the same +// layout, at least for the flags we know about. This isn't guaranteed, but is +// likely to remain true, and as long as it is true it makes conversions easy. +#define ASSERT_DIFLAG_VALUE(FLAG, VALUE) \ + static_assert((LLVMDI##FLAG == (VALUE)) && (DINode::DIFlags::FLAG == (VALUE))) +ASSERT_DIFLAG_VALUE(FlagZero, 0); +ASSERT_DIFLAG_VALUE(FlagPrivate, 1); +ASSERT_DIFLAG_VALUE(FlagProtected, 2); +ASSERT_DIFLAG_VALUE(FlagPublic, 3); +// Bit (1 << 1) is part of the private/protected/public values above. +ASSERT_DIFLAG_VALUE(FlagFwdDecl, 1 << 2); +ASSERT_DIFLAG_VALUE(FlagAppleBlock, 1 << 3); +ASSERT_DIFLAG_VALUE(FlagReservedBit4, 1 << 4); +ASSERT_DIFLAG_VALUE(FlagVirtual, 1 << 5); +ASSERT_DIFLAG_VALUE(FlagArtificial, 1 << 6); +ASSERT_DIFLAG_VALUE(FlagExplicit, 1 << 7); +ASSERT_DIFLAG_VALUE(FlagPrototyped, 1 << 8); +ASSERT_DIFLAG_VALUE(FlagObjcClassComplete, 1 << 9); +ASSERT_DIFLAG_VALUE(FlagObjectPointer, 1 << 10); +ASSERT_DIFLAG_VALUE(FlagVector, 1 << 11); +ASSERT_DIFLAG_VALUE(FlagStaticMember, 1 << 12); +ASSERT_DIFLAG_VALUE(FlagLValueReference, 1 << 13); +ASSERT_DIFLAG_VALUE(FlagRValueReference, 1 << 14); +// Bit (1 << 15) has been recycled, but the C API value hasn't been renamed. +static_assert((LLVMDIFlagReserved == (1 << 15)) && + (DINode::DIFlags::FlagExportSymbols == (1 << 15))); +ASSERT_DIFLAG_VALUE(FlagSingleInheritance, 1 << 16); +ASSERT_DIFLAG_VALUE(FlagMultipleInheritance, 2 << 16); +ASSERT_DIFLAG_VALUE(FlagVirtualInheritance, 3 << 16); +// Bit (1 << 17) is part of the inheritance values above. +ASSERT_DIFLAG_VALUE(FlagIntroducedVirtual, 1 << 18); +ASSERT_DIFLAG_VALUE(FlagBitField, 1 << 19); +ASSERT_DIFLAG_VALUE(FlagNoReturn, 1 << 20); +// Bit (1 << 21) is unused, but was `LLVMDIFlagMainSubprogram`. +ASSERT_DIFLAG_VALUE(FlagTypePassByValue, 1 << 22); +ASSERT_DIFLAG_VALUE(FlagTypePassByReference, 1 << 23); +ASSERT_DIFLAG_VALUE(FlagEnumClass, 1 << 24); +ASSERT_DIFLAG_VALUE(FlagThunk, 1 << 25); +ASSERT_DIFLAG_VALUE(FlagNonTrivial, 1 << 26); +ASSERT_DIFLAG_VALUE(FlagBigEndian, 1 << 27); +ASSERT_DIFLAG_VALUE(FlagLittleEndian, 1 << 28); +ASSERT_DIFLAG_VALUE(FlagIndirectVirtualBase, (1 << 2) | (1 << 5)); +#undef ASSERT_DIFLAG_VALUE + +// There are two potential ways to convert `LLVMDIFlags` to `DIFlags`: +// - Check and copy every individual bit/subvalue from input to output. +// - Statically assert that both have the same layout, and cast. +// As long as the static assertions succeed, a cast is easier and faster. +// In the (hopefully) unlikely event that the assertions do fail someday, and +// LLVM doesn't expose its own conversion function, we'll have to switch over +// to copying each bit/subvalue. +static DINode::DIFlags fromRust(LLVMDIFlags Flags) { + // Check that all set bits are covered by the static assertions above. + const unsigned UNKNOWN_BITS = (1 << 31) | (1 << 30) | (1 << 29) | (1 << 21); + if (Flags & UNKNOWN_BITS) { + report_fatal_error("bad LLVMDIFlags"); + } + + // As long as the static assertions are satisfied and no unknown bits are + // present, we can convert from `LLVMDIFlags` to `DIFlags` with a cast. + return static_cast(Flags); } // These values **must** match debuginfo::DISPFlags! They also *happen* From d10bdafa26a58356b6aeba18e6e07693157636df Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 6 Jan 2025 21:36:38 +1100 Subject: [PATCH 2/2] Note that cg_llvm's gimli should match the version used elsewhere --- compiler/rustc_codegen_llvm/Cargo.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compiler/rustc_codegen_llvm/Cargo.toml b/compiler/rustc_codegen_llvm/Cargo.toml index c44d1a5e5c220..94f21ac5f5742 100644 --- a/compiler/rustc_codegen_llvm/Cargo.toml +++ b/compiler/rustc_codegen_llvm/Cargo.toml @@ -9,6 +9,8 @@ test = false [dependencies] # tidy-alphabetical-start bitflags = "2.4.1" +# To avoid duplicate dependencies, this should match the version of gimli used +# by `rustc_codegen_ssa` via its `thorin-dwp` dependency. gimli = "0.30" itertools = "0.12" libc = "0.2"