[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
https://github.com/john-brawn-arm closed https://github.com/llvm/llvm-project/pull/85665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
john-brawn-arm wrote: After doing some more testing it turns out that DW_OP_bit_piece actually doesn't work when we have a signed bitfield, as both gdb and lldb treat the upper bits as zero. I think instead we have to do a sequence of operations to extract the relevant bits from the value. I've created #93990 for adding a DW_OP for doing this, with patches for using it in clang and making optimisations handle it on the way. https://github.com/llvm/llvm-project/pull/85665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
@@ -6178,10 +6178,16 @@ The current supported opcode vocabulary is limited: the last entry from the second last entry and appends the result to the expression stack. - ``DW_OP_plus_uconst, 93`` adds ``93`` to the working expression. -- ``DW_OP_LLVM_fragment, 16, 8`` specifies the offset and size (``16`` and ``8`` - here, respectively) of the variable fragment from the working expression. Note - that contrary to DW_OP_bit_piece, the offset is describing the location - within the described source variable. +- ``DW_OP_LLVM_fragment, 16, 8`` specifies that the top of the expression stack + is a fragment of the source language variable with the given offset and size + (``16`` and ``8`` here, respectively). Note that the offset and size are the + opposite way around to ``DW_OP_bit_piece``, and the offset is within the + source language variable. +- ``DW_OP_bit_piece, 8, 16`` specifies that the source language variable can be + found in the sequence of bits at the given size and offset (``8`` and ``16`` + here, respectively) within the top of the expression stack. Note that the + offset and size are the opposite way around to ``DW_OP_LLVM_fragment``, and the + offset is within the LLVM variable (if that's at the top of the stack). john-brawn-arm wrote: Ah, I see, I hadn't noticed that (I'd just read the description of DW_OP_bit_piece and hadn't read the general description of composite location descriptions). I had been understanding something like "DW_OP_reg0 RAX, DW_OP_bit_piece 0x3 0x0" to mean "this variable can be found in the 3 bits at offset 0 in RAX", but it actually means something closer to "the bottom 3 bits of this variable can be found in the 3 bits at offset 0 in RAX". I think this doesn't make much difference though if we restrict DW_OP_bit_piece in the llvm.dbg intrinsics to use it only for its ability to describe a value stored in part of a register, with the other bits not being described, as I think that's what the behaviour I've currently implemented is. Though maybe it would be better to instead use DW_OP_and and DW_OP_shr to extract the bits from the register. I'll think about this some more. https://github.com/llvm/llvm-project/pull/85665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
@@ -6178,10 +6178,16 @@ The current supported opcode vocabulary is limited: the last entry from the second last entry and appends the result to the expression stack. - ``DW_OP_plus_uconst, 93`` adds ``93`` to the working expression. -- ``DW_OP_LLVM_fragment, 16, 8`` specifies the offset and size (``16`` and ``8`` - here, respectively) of the variable fragment from the working expression. Note - that contrary to DW_OP_bit_piece, the offset is describing the location - within the described source variable. +- ``DW_OP_LLVM_fragment, 16, 8`` specifies that the top of the expression stack + is a fragment of the source language variable with the given offset and size + (``16`` and ``8`` here, respectively). Note that the offset and size are the + opposite way around to ``DW_OP_bit_piece``, and the offset is within the + source language variable. +- ``DW_OP_bit_piece, 8, 16`` specifies that the source language variable can be + found in the sequence of bits at the given size and offset (``8`` and ``16`` + here, respectively) within the top of the expression stack. Note that the + offset and size are the opposite way around to ``DW_OP_LLVM_fragment``, and the + offset is within the LLVM variable (if that's at the top of the stack). adrian-prantl wrote: > DW_OP_bit_piece is like every other standard dwarf expression Not it's not. It's a composite location description delimiter and cannot appear inside a DWARF expression. (See DWARF 5 section 2.6.1.2). That's why it's important that all LLVM algorithms that need to reason about fragments (the LLVM equivalent of composite location descriptions) are aware of them. https://github.com/llvm/llvm-project/pull/85665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
@@ -6178,10 +6178,16 @@ The current supported opcode vocabulary is limited: the last entry from the second last entry and appends the result to the expression stack. - ``DW_OP_plus_uconst, 93`` adds ``93`` to the working expression. -- ``DW_OP_LLVM_fragment, 16, 8`` specifies the offset and size (``16`` and ``8`` - here, respectively) of the variable fragment from the working expression. Note - that contrary to DW_OP_bit_piece, the offset is describing the location - within the described source variable. +- ``DW_OP_LLVM_fragment, 16, 8`` specifies that the top of the expression stack + is a fragment of the source language variable with the given offset and size + (``16`` and ``8`` here, respectively). Note that the offset and size are the + opposite way around to ``DW_OP_bit_piece``, and the offset is within the + source language variable. +- ``DW_OP_bit_piece, 8, 16`` specifies that the source language variable can be + found in the sequence of bits at the given size and offset (``8`` and ``16`` + here, respectively) within the top of the expression stack. Note that the + offset and size are the opposite way around to ``DW_OP_LLVM_fragment``, and the + offset is within the LLVM variable (if that's at the top of the stack). john-brawn-arm wrote: DW_OP_bit_piece is like every other standard dwarf expression: if we see a new location for a variable then the variable is now at that location. The current behaviour for DIExpression::fragmentsOverlap() is that it returns true when the expression isn't a fragment, and that's what we want for DW_OP_bit_piece. https://github.com/llvm/llvm-project/pull/85665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
https://github.com/adrian-prantl edited https://github.com/llvm/llvm-project/pull/85665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
@@ -6178,10 +6178,16 @@ The current supported opcode vocabulary is limited: the last entry from the second last entry and appends the result to the expression stack. - ``DW_OP_plus_uconst, 93`` adds ``93`` to the working expression. -- ``DW_OP_LLVM_fragment, 16, 8`` specifies the offset and size (``16`` and ``8`` - here, respectively) of the variable fragment from the working expression. Note - that contrary to DW_OP_bit_piece, the offset is describing the location - within the described source variable. +- ``DW_OP_LLVM_fragment, 16, 8`` specifies that the top of the expression stack + is a fragment of the source language variable with the given offset and size + (``16`` and ``8`` here, respectively). Note that the offset and size are the + opposite way around to ``DW_OP_bit_piece``, and the offset is within the + source language variable. +- ``DW_OP_bit_piece, 8, 16`` specifies that the source language variable can be + found in the sequence of bits at the given size and offset (``8`` and ``16`` + here, respectively) within the top of the expression stack. Note that the + offset and size are the opposite way around to ``DW_OP_LLVM_fragment``, and the + offset is within the LLVM variable (if that's at the top of the stack). adrian-prantl wrote: > it turns out in most of the places that getFragmentInfo is used it actually > is specific to DW_OP_LLVM_fragment and does the wrong thing for > DW_OP_bit_piece, so I think adding a separate getBitPieceInfo is simpler. I am mostly worried about the places that wont to answer questions like `DIExpression::fragmentsOverlap()` in order to determine whether a fragment (or bit_piece) should replace or augment another dbg.value or DBG_VALUE for the same variable. https://github.com/llvm/llvm-project/pull/85665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
@@ -6178,10 +6178,16 @@ The current supported opcode vocabulary is limited: the last entry from the second last entry and appends the result to the expression stack. - ``DW_OP_plus_uconst, 93`` adds ``93`` to the working expression. -- ``DW_OP_LLVM_fragment, 16, 8`` specifies the offset and size (``16`` and ``8`` - here, respectively) of the variable fragment from the working expression. Note - that contrary to DW_OP_bit_piece, the offset is describing the location - within the described source variable. +- ``DW_OP_LLVM_fragment, 16, 8`` specifies that the top of the expression stack + is a fragment of the source language variable with the given offset and size + (``16`` and ``8`` here, respectively). Note that the offset and size are the + opposite way around to ``DW_OP_bit_piece``, and the offset is within the + source language variable. +- ``DW_OP_bit_piece, 8, 16`` specifies that the source language variable can be + found in the sequence of bits at the given size and offset (``8`` and ``16`` + here, respectively) within the top of the expression stack. Note that the + offset and size are the opposite way around to ``DW_OP_LLVM_fragment``, and the + offset is within the LLVM variable (if that's at the top of the stack). john-brawn-arm wrote: > So that effectively implements option (1) that I outlined above. I think this > is a reasonable choice and consistent with how DWARF uses them. So if we do > this (and I think that's fine) we need make sure that the bit pieces are > correctly reported by DIExpression::getFragmentInfo() or else they will get > incorrectly treated by every pass that needs to determine overlapping > fragments. I'm handling this in the next part, which is SROA support (https://github.com/john-brawn-arm/llvm-project/tree/sroa_bit_piece), though it turns out in most of the places that getFragmentInfo is used it actually is specific to DW_OP_LLVM_fragment and does the wrong thing for DW_OP_bit_piece, so I think adding a separate getBitPieceInfo is simpler. > At the very least there should be tests that have LLVM IR that mix & matches > DW_OP_bit_piece and DW_OP_LLVM_fragment for the same variable and confirm > that this gets lowered into valid DWARF. Alternatively, if you want to make > quick progress, you can also say that we don't support this for now and > actively reject mixing the two within the same variable in the Verifyer. The > you'd need to make sure that passes like SROA that introduce new fragments > deal with this gracefully by creating undef locations. Currently if the same variable has a DW_OP_LLVM_fragment and anything else this causes assertion failures in DebugLocEntry::finalize in llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp. https://github.com/llvm/llvm-project/pull/85665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
@@ -6178,10 +6178,16 @@ The current supported opcode vocabulary is limited: the last entry from the second last entry and appends the result to the expression stack. - ``DW_OP_plus_uconst, 93`` adds ``93`` to the working expression. -- ``DW_OP_LLVM_fragment, 16, 8`` specifies the offset and size (``16`` and ``8`` - here, respectively) of the variable fragment from the working expression. Note - that contrary to DW_OP_bit_piece, the offset is describing the location - within the described source variable. +- ``DW_OP_LLVM_fragment, 16, 8`` specifies that the top of the expression stack + is a fragment of the source language variable with the given offset and size + (``16`` and ``8`` here, respectively). Note that the offset and size are the + opposite way around to ``DW_OP_bit_piece``, and the offset is within the + source language variable. +- ``DW_OP_bit_piece, 8, 16`` specifies that the source language variable can be + found in the sequence of bits at the given size and offset (``8`` and ``16`` + here, respectively) within the top of the expression stack. Note that the + offset and size are the opposite way around to ``DW_OP_LLVM_fragment``, and the + offset is within the LLVM variable (if that's at the top of the stack). adrian-prantl wrote: So that effectively implements option (1) that I outlined above. I think this is a reasonable choice and consistent with how DWARF uses them. So if we do this (and I think that's fine) we need make sure that the bit pieces are correctly reported by DIExpression::getFragmentInfo() or else they will get incorrectly treated by every pass that needs to determine overlapping fragments. At the very least there should be tests that have LLVM IR that mix & matches DW_OP_bit_piece and DW_OP_LLVM_fragment for the same variable and confirm that this gets lowered into valid DWARF. Alternatively, if you want to make quick progress, you can also say that we don't support this for now and actively reject mixing the two within the same variable in the Verifyer. The you'd need to make sure that passes like SROA that introduce new fragments deal with this gracefully by creating undef locations. https://github.com/llvm/llvm-project/pull/85665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
@@ -6178,10 +6178,16 @@ The current supported opcode vocabulary is limited: the last entry from the second last entry and appends the result to the expression stack. - ``DW_OP_plus_uconst, 93`` adds ``93`` to the working expression. -- ``DW_OP_LLVM_fragment, 16, 8`` specifies the offset and size (``16`` and ``8`` - here, respectively) of the variable fragment from the working expression. Note - that contrary to DW_OP_bit_piece, the offset is describing the location - within the described source variable. +- ``DW_OP_LLVM_fragment, 16, 8`` specifies that the top of the expression stack + is a fragment of the source language variable with the given offset and size + (``16`` and ``8`` here, respectively). Note that the offset and size are the + opposite way around to ``DW_OP_bit_piece``, and the offset is within the + source language variable. +- ``DW_OP_bit_piece, 8, 16`` specifies that the source language variable can be + found in the sequence of bits at the given size and offset (``8`` and ``16`` + here, respectively) within the top of the expression stack. Note that the + offset and size are the opposite way around to ``DW_OP_LLVM_fragment``, and the + offset is within the LLVM variable (if that's at the top of the stack). john-brawn-arm wrote: There's already a verifier check that DW_OP_LLVM_fragment is the last expression in the list, so I've made DW_OP_bit_piece do the same. https://github.com/llvm/llvm-project/pull/85665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
https://github.com/john-brawn-arm updated
https://github.com/llvm/llvm-project/pull/85665
>From 665d4034a1428d9b5cf1c4d4e89a16fa00b94fb5 Mon Sep 17 00:00:00 2001
From: John Brawn
Date: Thu, 14 Mar 2024 16:17:03 +
Subject: [PATCH 1/4] [DebugInfo] Use DW_op_bit_piece for structured bindings
of bitfields
Currently we use DW_OP_plus_uconst to handle the bitfield offset and
handle the bitfield size by choosing a type size that matches, but
this doesn't work if either offset or size aren't byte-aligned. Using
DW_op_bit_piece means we can handle any offset and size.
---
clang/lib/CodeGen/CGDebugInfo.cpp | 52 ++--
clang/lib/CodeGen/CGDebugInfo.h | 3 -
...debug-info-structured-binding-bitfield.cpp | 118 +-
.../CodeGen/AsmPrinter/DwarfExpression.cpp| 3 +
llvm/lib/IR/DebugInfoMetadata.cpp | 2 +
5 files changed, 70 insertions(+), 108 deletions(-)
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp
b/clang/lib/CodeGen/CGDebugInfo.cpp
index c2c01439f2dc9..bf142bdbbe0f9 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4777,40 +4777,6 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const
VarDecl *VD,
return D;
}
-llvm::DIType *CGDebugInfo::CreateBindingDeclType(const BindingDecl *BD) {
- llvm::DIFile *Unit = getOrCreateFile(BD->getLocation());
-
- // If the declaration is bound to a bitfield struct field, its type may have
a
- // size that is different from its deduced declaration type's.
- if (const MemberExpr *ME = dyn_cast(BD->getBinding())) {
-if (const FieldDecl *FD = dyn_cast(ME->getMemberDecl())) {
- if (FD->isBitField()) {
-ASTContext = CGM.getContext();
-const CGRecordLayout =
-CGM.getTypes().getCGRecordLayout(FD->getParent());
-const CGBitFieldInfo = RL.getBitFieldInfo(FD);
-
-// Find an integer type with the same bitwidth as the bitfield size. If
-// no suitable type is present in the target, give up on producing
debug
-// information as it would be wrong. It is certainly possible to
produce
-// correct debug info, but the logic isn't currently implemented.
-uint64_t BitfieldSizeInBits = Info.Size;
-QualType IntTy =
-Context.getIntTypeForBitwidth(BitfieldSizeInBits, Info.IsSigned);
-if (IntTy.isNull())
- return nullptr;
-Qualifiers Quals = BD->getType().getQualifiers();
-QualType FinalTy = Context.getQualifiedType(IntTy, Quals);
-llvm::DIType *Ty = getOrCreateType(FinalTy, Unit);
-assert(Ty);
-return Ty;
- }
-}
- }
-
- return getOrCreateType(BD->getType(), Unit);
-}
-
llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const BindingDecl *BD,
llvm::Value *Storage,
std::optional ArgNo,
@@ -4825,7 +4791,8 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const
BindingDecl *BD,
if (isa(BD->getBinding()))
return nullptr;
- llvm::DIType *Ty = CreateBindingDeclType(BD);
+ llvm::DIFile *Unit = getOrCreateFile(BD->getLocation());
+ llvm::DIType *Ty = getOrCreateType(BD->getType(), Unit);
// If there is no debug info for this type then do not emit debug info
// for this variable.
@@ -4851,7 +4818,6 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const
BindingDecl *BD,
unsigned Column = getColumnNumber(BD->getLocation());
StringRef Name = BD->getName();
auto *Scope = cast(LexicalBlockStack.back());
- llvm::DIFile *Unit = getOrCreateFile(BD->getLocation());
// Create the descriptor for the variable.
llvm::DILocalVariable *D = DBuilder.createAutoVariable(
Scope, Name, Unit, Line, Ty, CGM.getLangOpts().Optimize,
@@ -4865,13 +4831,13 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const
BindingDecl *BD,
const ASTRecordLayout =
CGM.getContext().getASTRecordLayout(parent);
const uint64_t fieldOffset = layout.getFieldOffset(fieldIndex);
-
- if (fieldOffset != 0) {
-// Currently if the field offset is not a multiple of byte, the
produced
-// location would not be accurate. Therefore give up.
-if (fieldOffset % CGM.getContext().getCharWidth() != 0)
- return nullptr;
-
+ if (FD->isBitField()) {
+Expr.push_back(llvm::dwarf::DW_OP_bit_piece);
+Expr.push_back(FD->getBitWidthValue(CGM.getContext()));
+Expr.push_back(fieldOffset);
+ } else if (fieldOffset != 0) {
+assert(fieldOffset % CGM.getContext().getCharWidth() == 0 &&
+ "Unexpected non-bitfield with non-byte-aligned offset");
Expr.push_back(llvm::dwarf::DW_OP_plus_uconst);
Expr.push_back(
CGM.getContext().toCharUnitsFromBits(fieldOffset).getQuantity());
diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h
index 7b60e94555d06..af58ccdb5b35d 100644
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
@@ -6178,10 +6178,16 @@ The current supported opcode vocabulary is limited: the last entry from the second last entry and appends the result to the expression stack. - ``DW_OP_plus_uconst, 93`` adds ``93`` to the working expression. -- ``DW_OP_LLVM_fragment, 16, 8`` specifies the offset and size (``16`` and ``8`` - here, respectively) of the variable fragment from the working expression. Note - that contrary to DW_OP_bit_piece, the offset is describing the location - within the described source variable. +- ``DW_OP_LLVM_fragment, 16, 8`` specifies that the top of the expression stack + is a fragment of the source language variable with the given offset and size + (``16`` and ``8`` here, respectively). Note that the offset and size are the + opposite way around to ``DW_OP_bit_piece``, and the offset is within the + source language variable. +- ``DW_OP_bit_piece, 8, 16`` specifies that the source language variable can be + found in the sequence of bits at the given size and offset (``8`` and ``16`` + here, respectively) within the top of the expression stack. Note that the + offset and size are the opposite way around to ``DW_OP_LLVM_fragment``, and the + offset is within the LLVM variable (if that's at the top of the stack). adrian-prantl wrote: One thing that is missing right now is discussion whether DW_OP_bit_piece can be used inside of a DW_OP_LLVM_fragment expression. - If a DW_OP_bit_piece generates a fragment on its own, then combining the two should be made illegal in the verifier, and then it needs to be handled everywhere we handle DW_OP_LLVM_fragment. - If the two can be combined then DWARFExpression needs handle that case, and we probably need a few more testcases for all the edge cases. https://github.com/llvm/llvm-project/pull/85665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
@@ -6178,10 +6178,16 @@ The current supported opcode vocabulary is limited: the last entry from the second last entry and appends the result to the expression stack. - ``DW_OP_plus_uconst, 93`` adds ``93`` to the working expression. -- ``DW_OP_LLVM_fragment, 16, 8`` specifies the offset and size (``16`` and ``8`` - here, respectively) of the variable fragment from the working expression. Note - that contrary to DW_OP_bit_piece, the offset is describing the location - within the described source variable. +- ``DW_OP_LLVM_fragment, 16, 8`` specifies that the top of the expression stack + is a fragment of the source language variable with the given offset and size + (``16`` and ``8`` here, respectively). Note that the offset and size are the + opposite way around to ``DW_OP_bit_piece``, and the offset is within the + source language variable. +- ``DW_OP_bit_piece, 8, 16`` specifies that the source language variable can be + found in the sequence of bits at the given size and offset (``8`` and ``16`` + here, respectively) within the top of the expression stack. Note that the + offset and size are the opposite way around to ``DW_OP_LLVM_fragment``, and the + offset is within the LLVM variable (if that's at the top of the stack). adrian-prantl wrote: I guess we need more testcases for the edge cases regardless of which way we decide. https://github.com/llvm/llvm-project/pull/85665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
https://github.com/john-brawn-arm updated
https://github.com/llvm/llvm-project/pull/85665
>From 665d4034a1428d9b5cf1c4d4e89a16fa00b94fb5 Mon Sep 17 00:00:00 2001
From: John Brawn
Date: Thu, 14 Mar 2024 16:17:03 +
Subject: [PATCH 1/3] [DebugInfo] Use DW_op_bit_piece for structured bindings
of bitfields
Currently we use DW_OP_plus_uconst to handle the bitfield offset and
handle the bitfield size by choosing a type size that matches, but
this doesn't work if either offset or size aren't byte-aligned. Using
DW_op_bit_piece means we can handle any offset and size.
---
clang/lib/CodeGen/CGDebugInfo.cpp | 52 ++--
clang/lib/CodeGen/CGDebugInfo.h | 3 -
...debug-info-structured-binding-bitfield.cpp | 118 +-
.../CodeGen/AsmPrinter/DwarfExpression.cpp| 3 +
llvm/lib/IR/DebugInfoMetadata.cpp | 2 +
5 files changed, 70 insertions(+), 108 deletions(-)
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp
b/clang/lib/CodeGen/CGDebugInfo.cpp
index c2c01439f2dc9..bf142bdbbe0f9 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4777,40 +4777,6 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const
VarDecl *VD,
return D;
}
-llvm::DIType *CGDebugInfo::CreateBindingDeclType(const BindingDecl *BD) {
- llvm::DIFile *Unit = getOrCreateFile(BD->getLocation());
-
- // If the declaration is bound to a bitfield struct field, its type may have
a
- // size that is different from its deduced declaration type's.
- if (const MemberExpr *ME = dyn_cast(BD->getBinding())) {
-if (const FieldDecl *FD = dyn_cast(ME->getMemberDecl())) {
- if (FD->isBitField()) {
-ASTContext = CGM.getContext();
-const CGRecordLayout =
-CGM.getTypes().getCGRecordLayout(FD->getParent());
-const CGBitFieldInfo = RL.getBitFieldInfo(FD);
-
-// Find an integer type with the same bitwidth as the bitfield size. If
-// no suitable type is present in the target, give up on producing
debug
-// information as it would be wrong. It is certainly possible to
produce
-// correct debug info, but the logic isn't currently implemented.
-uint64_t BitfieldSizeInBits = Info.Size;
-QualType IntTy =
-Context.getIntTypeForBitwidth(BitfieldSizeInBits, Info.IsSigned);
-if (IntTy.isNull())
- return nullptr;
-Qualifiers Quals = BD->getType().getQualifiers();
-QualType FinalTy = Context.getQualifiedType(IntTy, Quals);
-llvm::DIType *Ty = getOrCreateType(FinalTy, Unit);
-assert(Ty);
-return Ty;
- }
-}
- }
-
- return getOrCreateType(BD->getType(), Unit);
-}
-
llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const BindingDecl *BD,
llvm::Value *Storage,
std::optional ArgNo,
@@ -4825,7 +4791,8 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const
BindingDecl *BD,
if (isa(BD->getBinding()))
return nullptr;
- llvm::DIType *Ty = CreateBindingDeclType(BD);
+ llvm::DIFile *Unit = getOrCreateFile(BD->getLocation());
+ llvm::DIType *Ty = getOrCreateType(BD->getType(), Unit);
// If there is no debug info for this type then do not emit debug info
// for this variable.
@@ -4851,7 +4818,6 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const
BindingDecl *BD,
unsigned Column = getColumnNumber(BD->getLocation());
StringRef Name = BD->getName();
auto *Scope = cast(LexicalBlockStack.back());
- llvm::DIFile *Unit = getOrCreateFile(BD->getLocation());
// Create the descriptor for the variable.
llvm::DILocalVariable *D = DBuilder.createAutoVariable(
Scope, Name, Unit, Line, Ty, CGM.getLangOpts().Optimize,
@@ -4865,13 +4831,13 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const
BindingDecl *BD,
const ASTRecordLayout =
CGM.getContext().getASTRecordLayout(parent);
const uint64_t fieldOffset = layout.getFieldOffset(fieldIndex);
-
- if (fieldOffset != 0) {
-// Currently if the field offset is not a multiple of byte, the
produced
-// location would not be accurate. Therefore give up.
-if (fieldOffset % CGM.getContext().getCharWidth() != 0)
- return nullptr;
-
+ if (FD->isBitField()) {
+Expr.push_back(llvm::dwarf::DW_OP_bit_piece);
+Expr.push_back(FD->getBitWidthValue(CGM.getContext()));
+Expr.push_back(fieldOffset);
+ } else if (fieldOffset != 0) {
+assert(fieldOffset % CGM.getContext().getCharWidth() == 0 &&
+ "Unexpected non-bitfield with non-byte-aligned offset");
Expr.push_back(llvm::dwarf::DW_OP_plus_uconst);
Expr.push_back(
CGM.getContext().toCharUnitsFromBits(fieldOffset).getQuantity());
diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h
index 7b60e94555d06..af58ccdb5b35d 100644
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
jryans wrote: It would be great to add a few notes to document this additional op in the [`DIExpression` list of ops](https://llvm.org/docs/LangRef.html#diexpression). It would be especially good for those added docs to clarify the difference between this and `DW_OP_LLVM_fragment`. (I do understand that this `DW_OP_bit_piece` op is part of standard DWARF, and there are already some standard DWARF ops we use that are missing from those docs already, but even with that in mind ... it would be great to have new docs added for this additional op.) https://github.com/llvm/llvm-project/pull/85665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
https://github.com/john-brawn-arm updated
https://github.com/llvm/llvm-project/pull/85665
>From 665d4034a1428d9b5cf1c4d4e89a16fa00b94fb5 Mon Sep 17 00:00:00 2001
From: John Brawn
Date: Thu, 14 Mar 2024 16:17:03 +
Subject: [PATCH 1/2] [DebugInfo] Use DW_op_bit_piece for structured bindings
of bitfields
Currently we use DW_OP_plus_uconst to handle the bitfield offset and
handle the bitfield size by choosing a type size that matches, but
this doesn't work if either offset or size aren't byte-aligned. Using
DW_op_bit_piece means we can handle any offset and size.
---
clang/lib/CodeGen/CGDebugInfo.cpp | 52 ++--
clang/lib/CodeGen/CGDebugInfo.h | 3 -
...debug-info-structured-binding-bitfield.cpp | 118 +-
.../CodeGen/AsmPrinter/DwarfExpression.cpp| 3 +
llvm/lib/IR/DebugInfoMetadata.cpp | 2 +
5 files changed, 70 insertions(+), 108 deletions(-)
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp
b/clang/lib/CodeGen/CGDebugInfo.cpp
index c2c01439f2dc9..bf142bdbbe0f9 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4777,40 +4777,6 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const
VarDecl *VD,
return D;
}
-llvm::DIType *CGDebugInfo::CreateBindingDeclType(const BindingDecl *BD) {
- llvm::DIFile *Unit = getOrCreateFile(BD->getLocation());
-
- // If the declaration is bound to a bitfield struct field, its type may have
a
- // size that is different from its deduced declaration type's.
- if (const MemberExpr *ME = dyn_cast(BD->getBinding())) {
-if (const FieldDecl *FD = dyn_cast(ME->getMemberDecl())) {
- if (FD->isBitField()) {
-ASTContext = CGM.getContext();
-const CGRecordLayout =
-CGM.getTypes().getCGRecordLayout(FD->getParent());
-const CGBitFieldInfo = RL.getBitFieldInfo(FD);
-
-// Find an integer type with the same bitwidth as the bitfield size. If
-// no suitable type is present in the target, give up on producing
debug
-// information as it would be wrong. It is certainly possible to
produce
-// correct debug info, but the logic isn't currently implemented.
-uint64_t BitfieldSizeInBits = Info.Size;
-QualType IntTy =
-Context.getIntTypeForBitwidth(BitfieldSizeInBits, Info.IsSigned);
-if (IntTy.isNull())
- return nullptr;
-Qualifiers Quals = BD->getType().getQualifiers();
-QualType FinalTy = Context.getQualifiedType(IntTy, Quals);
-llvm::DIType *Ty = getOrCreateType(FinalTy, Unit);
-assert(Ty);
-return Ty;
- }
-}
- }
-
- return getOrCreateType(BD->getType(), Unit);
-}
-
llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const BindingDecl *BD,
llvm::Value *Storage,
std::optional ArgNo,
@@ -4825,7 +4791,8 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const
BindingDecl *BD,
if (isa(BD->getBinding()))
return nullptr;
- llvm::DIType *Ty = CreateBindingDeclType(BD);
+ llvm::DIFile *Unit = getOrCreateFile(BD->getLocation());
+ llvm::DIType *Ty = getOrCreateType(BD->getType(), Unit);
// If there is no debug info for this type then do not emit debug info
// for this variable.
@@ -4851,7 +4818,6 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const
BindingDecl *BD,
unsigned Column = getColumnNumber(BD->getLocation());
StringRef Name = BD->getName();
auto *Scope = cast(LexicalBlockStack.back());
- llvm::DIFile *Unit = getOrCreateFile(BD->getLocation());
// Create the descriptor for the variable.
llvm::DILocalVariable *D = DBuilder.createAutoVariable(
Scope, Name, Unit, Line, Ty, CGM.getLangOpts().Optimize,
@@ -4865,13 +4831,13 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const
BindingDecl *BD,
const ASTRecordLayout =
CGM.getContext().getASTRecordLayout(parent);
const uint64_t fieldOffset = layout.getFieldOffset(fieldIndex);
-
- if (fieldOffset != 0) {
-// Currently if the field offset is not a multiple of byte, the
produced
-// location would not be accurate. Therefore give up.
-if (fieldOffset % CGM.getContext().getCharWidth() != 0)
- return nullptr;
-
+ if (FD->isBitField()) {
+Expr.push_back(llvm::dwarf::DW_OP_bit_piece);
+Expr.push_back(FD->getBitWidthValue(CGM.getContext()));
+Expr.push_back(fieldOffset);
+ } else if (fieldOffset != 0) {
+assert(fieldOffset % CGM.getContext().getCharWidth() == 0 &&
+ "Unexpected non-bitfield with non-byte-aligned offset");
Expr.push_back(llvm::dwarf::DW_OP_plus_uconst);
Expr.push_back(
CGM.getContext().toCharUnitsFromBits(fieldOffset).getQuantity());
diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h
index 7b60e94555d06..af58ccdb5b35d 100644
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
https://github.com/pogo59 commented: The LLVM changes don't have a test. I expect there are existing expression-to-final-DWARF tests that you can modify to throw in a few bit-piece operators and show they come out correctly. Offhand it seems quite reasonable to use DW_OP_bit_piece for bitfields. https://github.com/llvm/llvm-project/pull/85665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
john-brawn-arm wrote: Ping. https://github.com/llvm/llvm-project/pull/85665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
@@ -8,8 +8,8 @@ struct S0 {
// CHECK-LABEL: define dso_local void @_Z3fS0v
// CHECK:alloca %struct.S0, align 4
// CHECK-NEXT:[[TMP0:%.*]] = alloca %struct.S0, align 4
-// CHECK: call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_A:![0-9]+]], metadata !DIExpression())
-// CHECK-NEXT:call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_B:![0-9]+]], metadata !DIExpression(DW_OP_plus_uconst, 2))
+// CHECK: call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_A:![0-9]+]], metadata !DIExpression(DW_OP_bit_piece, 16, 0))
john-brawn-arm wrote:
https://github.com/john-brawn-arm/llvm-project/tree/sroa_bit_piece makes sroa
work with bit_piece, I'll create a pull requeset once this on is merged.
https://github.com/llvm/llvm-project/pull/85665
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
@@ -8,8 +8,8 @@ struct S0 {
// CHECK-LABEL: define dso_local void @_Z3fS0v
// CHECK:alloca %struct.S0, align 4
// CHECK-NEXT:[[TMP0:%.*]] = alloca %struct.S0, align 4
-// CHECK: call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_A:![0-9]+]], metadata !DIExpression())
-// CHECK-NEXT:call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_B:![0-9]+]], metadata !DIExpression(DW_OP_plus_uconst, 2))
+// CHECK: call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_A:![0-9]+]], metadata !DIExpression(DW_OP_bit_piece, 16, 0))
john-brawn-arm wrote:
> So we could add DW_OP_bit_piece to LLVM, or we could emit
>
> ```
> !DIExpression()
> !DIExpression(DW_OP_shr 8)
> ```
>
> Which, I believe, would be supported by LLVM today without any modifications?
That would handle the bitfield offset, but wouldn't get the bitfield size right
(we could use the existing CreateBindingDeclType but that can only handle cases
where the bitfield size exactly matches an integer type size).
> Regardless of which variant we choose, we probably need to add explicit
> support for it to SROA.cpp to make sure it gets translated correctly when the
> alloca gets split up again.
I'm currently working on this, should have a patch for it soon (though it looks
like we only need changes in DIExpression::createFragmentExpression).
https://github.com/llvm/llvm-project/pull/85665
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
@@ -8,8 +8,8 @@ struct S0 {
// CHECK-LABEL: define dso_local void @_Z3fS0v
// CHECK:alloca %struct.S0, align 4
// CHECK-NEXT:[[TMP0:%.*]] = alloca %struct.S0, align 4
-// CHECK: call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_A:![0-9]+]], metadata !DIExpression())
-// CHECK-NEXT:call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_B:![0-9]+]], metadata !DIExpression(DW_OP_plus_uconst, 2))
+// CHECK: call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_A:![0-9]+]], metadata !DIExpression(DW_OP_bit_piece, 16, 0))
adrian-prantl wrote:
The SROA support doesn't need to hold up this patch of course, I'm just
mentioning it.
https://github.com/llvm/llvm-project/pull/85665
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
@@ -8,8 +8,8 @@ struct S0 {
// CHECK-LABEL: define dso_local void @_Z3fS0v
// CHECK:alloca %struct.S0, align 4
// CHECK-NEXT:[[TMP0:%.*]] = alloca %struct.S0, align 4
-// CHECK: call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_A:![0-9]+]], metadata !DIExpression())
-// CHECK-NEXT:call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_B:![0-9]+]], metadata !DIExpression(DW_OP_plus_uconst, 2))
+// CHECK: call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_A:![0-9]+]], metadata !DIExpression(DW_OP_bit_piece, 16, 0))
adrian-prantl wrote:
Regardless of which variant we choose, we probably need to add explicit support
for it to SROA.cpp to make sure it gets translated correctly when the alloca
gets split up again.
https://github.com/llvm/llvm-project/pull/85665
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
@@ -8,8 +8,8 @@ struct S0 {
// CHECK-LABEL: define dso_local void @_Z3fS0v
// CHECK:alloca %struct.S0, align 4
// CHECK-NEXT:[[TMP0:%.*]] = alloca %struct.S0, align 4
-// CHECK: call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_A:![0-9]+]], metadata !DIExpression())
-// CHECK-NEXT:call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_B:![0-9]+]], metadata !DIExpression(DW_OP_plus_uconst, 2))
+// CHECK: call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_A:![0-9]+]], metadata !DIExpression(DW_OP_bit_piece, 16, 0))
adrian-prantl wrote:
https://dwarfstd.org/issues/191025.1.html
https://dwarfstd.org/issues/161206.2.html
I think you are right about the semantics of `DW_OP_bit_piece`.
I also think I misunderstood the problem the first time around.
Here a and b are not fragments of a large struct, but we want to express the
opposite. They point into the same alloca. Here's incorrect ToT output:
https://github.com/llvm/llvm-project/pull/85665
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
@@ -8,8 +8,8 @@ struct S0 {
// CHECK-LABEL: define dso_local void @_Z3fS0v
// CHECK:alloca %struct.S0, align 4
// CHECK-NEXT:[[TMP0:%.*]] = alloca %struct.S0, align 4
-// CHECK: call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_A:![0-9]+]], metadata !DIExpression())
-// CHECK-NEXT:call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_B:![0-9]+]], metadata !DIExpression(DW_OP_plus_uconst, 2))
+// CHECK: call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_A:![0-9]+]], metadata !DIExpression(DW_OP_bit_piece, 16, 0))
adrian-prantl wrote:
So we could add DW_OP_bit_piece to LLVM, or we could emit
```
!DIExpression()
!DIExpression(DW_OP_shr 8)
```
Which, I believe, would be supported by LLVM today without any modifications?
https://github.com/llvm/llvm-project/pull/85665
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
@@ -8,8 +8,8 @@ struct S0 {
// CHECK-LABEL: define dso_local void @_Z3fS0v
// CHECK:alloca %struct.S0, align 4
// CHECK-NEXT:[[TMP0:%.*]] = alloca %struct.S0, align 4
-// CHECK: call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_A:![0-9]+]], metadata !DIExpression())
-// CHECK-NEXT:call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_B:![0-9]+]], metadata !DIExpression(DW_OP_plus_uconst, 2))
+// CHECK: call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_A:![0-9]+]], metadata !DIExpression(DW_OP_bit_piece, 16, 0))
adrian-prantl wrote:
```
%0 = alloca %struct.anon, align 4
store i32 0, ptr %retval, align 4
tail call void @llvm.dbg.declare(metadata ptr %0, metadata !23, metadata
!DIExpression()), !dbg !25
tail call void @llvm.dbg.declare(metadata ptr %0, metadata !26, metadata
!DIExpression(DW_OP_plus_uconst, 1)), !dbg !27
...
!23 = !DILocalVariable(name: "a", scope: !18, file: !5, line: 2, type: !24)
!26 = !DILocalVariable(name: "b", scope: !18, file: !5, line: 2, type: !24)
```
https://github.com/llvm/llvm-project/pull/85665
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
@@ -8,8 +8,8 @@ struct S0 {
// CHECK-LABEL: define dso_local void @_Z3fS0v
// CHECK:alloca %struct.S0, align 4
// CHECK-NEXT:[[TMP0:%.*]] = alloca %struct.S0, align 4
-// CHECK: call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_A:![0-9]+]], metadata !DIExpression())
-// CHECK-NEXT:call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_B:![0-9]+]], metadata !DIExpression(DW_OP_plus_uconst, 2))
+// CHECK: call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_A:![0-9]+]], metadata !DIExpression(DW_OP_bit_piece, 16, 0))
john-brawn-arm wrote:
The LLVM langref says "Note that contrary to DW_OP_bit_piece, the offset is
describing the location within the described source variable". What I think
this means is that while DW_OP_bit_piece means "you get the source variable by
extracting these bits of this location", DW_OP_LLVM_fragment means "the value
of this location is these bits of the source variable".
Trying out DW_OP_LLVM_fragment in
```
struct {
unsigned int a : 8;
unsigned int b : 8;
} X = { 1, 2 };
int main() {
auto [a, b] = X;
return a + b;
}
```
the resulting dwarf info looks pretty strange
```
<2><7f>: Abbrev Number: 7 (DW_TAG_variable)
<80> DW_AT_location: 4 byte block: 93 1 91 78 (DW_OP_piece:
1; DW_OP_fbreg: -8)
<85> DW_AT_name: (indirect string, offset: 0x9c): a
<89> DW_AT_decl_file : 1
<8a> DW_AT_decl_line : 7
<8b> DW_AT_type: <0x5f>
<2><8f>: Abbrev Number: 7 (DW_TAG_variable)
<90> DW_AT_location: 6 byte block: 93 1 91 78 93 1(DW_OP_piece:
1; DW_OP_fbreg: -8; DW_OP_piece: 1)
<97> DW_AT_name: (indirect string, offset: 0xab): b
<9b> DW_AT_decl_file : 1
<9c> DW_AT_decl_line : 7
<9d> DW_AT_type: <0x5f>
```
lldb says (at line 8)
```
(lldb) p a
(unsigned int) 513
(lldb) p b
(unsigned int) 256
```
gdb appears to not like this dwarf info and just says
```
(gdb) p a
$1 =
(gdb) p b
$2 =
```
https://github.com/llvm/llvm-project/pull/85665
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
@@ -8,8 +8,8 @@ struct S0 {
// CHECK-LABEL: define dso_local void @_Z3fS0v
// CHECK:alloca %struct.S0, align 4
// CHECK-NEXT:[[TMP0:%.*]] = alloca %struct.S0, align 4
-// CHECK: call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_A:![0-9]+]], metadata !DIExpression())
-// CHECK-NEXT:call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_B:![0-9]+]], metadata !DIExpression(DW_OP_plus_uconst, 2))
+// CHECK: call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata
[[S0_A:![0-9]+]], metadata !DIExpression(DW_OP_bit_piece, 16, 0))
adrian-prantl wrote:
We generally don't use DW_OP_bit_piece directly in LLVM IR. The correct thing
to do is to use DW_OP_LLVM_fragment instead, which then gets lowered to a
bit_piece in AsmPrinter.
https://github.com/llvm/llvm-project/pull/85665
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
https://github.com/adrian-prantl edited https://github.com/llvm/llvm-project/pull/85665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
https://github.com/adrian-prantl requested changes to this pull request. https://github.com/llvm/llvm-project/pull/85665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
llvmbot wrote:
@llvm/pr-subscribers-llvm-ir
@llvm/pr-subscribers-clang
Author: John Brawn (john-brawn-arm)
Changes
Currently we use DW_OP_plus_uconst to handle the bitfield offset and handle the
bitfield size by choosing a type size that matches, but this doesn't work if
either offset or size aren't byte-aligned. Using DW_op_bit_piece means we can
handle any offset and size.
---
Patch is 23.89 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/85665.diff
5 Files Affected:
- (modified) clang/lib/CodeGen/CGDebugInfo.cpp (+9-43)
- (modified) clang/lib/CodeGen/CGDebugInfo.h (-3)
- (modified) clang/test/CodeGenCXX/debug-info-structured-binding-bitfield.cpp
(+56-62)
- (modified) llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp (+3)
- (modified) llvm/lib/IR/DebugInfoMetadata.cpp (+2)
``diff
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp
b/clang/lib/CodeGen/CGDebugInfo.cpp
index c2c01439f2dc99..bf142bdbbe0f99 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4777,40 +4777,6 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const
VarDecl *VD,
return D;
}
-llvm::DIType *CGDebugInfo::CreateBindingDeclType(const BindingDecl *BD) {
- llvm::DIFile *Unit = getOrCreateFile(BD->getLocation());
-
- // If the declaration is bound to a bitfield struct field, its type may have
a
- // size that is different from its deduced declaration type's.
- if (const MemberExpr *ME = dyn_cast(BD->getBinding())) {
-if (const FieldDecl *FD = dyn_cast(ME->getMemberDecl())) {
- if (FD->isBitField()) {
-ASTContext = CGM.getContext();
-const CGRecordLayout =
-CGM.getTypes().getCGRecordLayout(FD->getParent());
-const CGBitFieldInfo = RL.getBitFieldInfo(FD);
-
-// Find an integer type with the same bitwidth as the bitfield size. If
-// no suitable type is present in the target, give up on producing
debug
-// information as it would be wrong. It is certainly possible to
produce
-// correct debug info, but the logic isn't currently implemented.
-uint64_t BitfieldSizeInBits = Info.Size;
-QualType IntTy =
-Context.getIntTypeForBitwidth(BitfieldSizeInBits, Info.IsSigned);
-if (IntTy.isNull())
- return nullptr;
-Qualifiers Quals = BD->getType().getQualifiers();
-QualType FinalTy = Context.getQualifiedType(IntTy, Quals);
-llvm::DIType *Ty = getOrCreateType(FinalTy, Unit);
-assert(Ty);
-return Ty;
- }
-}
- }
-
- return getOrCreateType(BD->getType(), Unit);
-}
-
llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const BindingDecl *BD,
llvm::Value *Storage,
std::optional ArgNo,
@@ -4825,7 +4791,8 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const
BindingDecl *BD,
if (isa(BD->getBinding()))
return nullptr;
- llvm::DIType *Ty = CreateBindingDeclType(BD);
+ llvm::DIFile *Unit = getOrCreateFile(BD->getLocation());
+ llvm::DIType *Ty = getOrCreateType(BD->getType(), Unit);
// If there is no debug info for this type then do not emit debug info
// for this variable.
@@ -4851,7 +4818,6 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const
BindingDecl *BD,
unsigned Column = getColumnNumber(BD->getLocation());
StringRef Name = BD->getName();
auto *Scope = cast(LexicalBlockStack.back());
- llvm::DIFile *Unit = getOrCreateFile(BD->getLocation());
// Create the descriptor for the variable.
llvm::DILocalVariable *D = DBuilder.createAutoVariable(
Scope, Name, Unit, Line, Ty, CGM.getLangOpts().Optimize,
@@ -4865,13 +4831,13 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const
BindingDecl *BD,
const ASTRecordLayout =
CGM.getContext().getASTRecordLayout(parent);
const uint64_t fieldOffset = layout.getFieldOffset(fieldIndex);
-
- if (fieldOffset != 0) {
-// Currently if the field offset is not a multiple of byte, the
produced
-// location would not be accurate. Therefore give up.
-if (fieldOffset % CGM.getContext().getCharWidth() != 0)
- return nullptr;
-
+ if (FD->isBitField()) {
+Expr.push_back(llvm::dwarf::DW_OP_bit_piece);
+Expr.push_back(FD->getBitWidthValue(CGM.getContext()));
+Expr.push_back(fieldOffset);
+ } else if (fieldOffset != 0) {
+assert(fieldOffset % CGM.getContext().getCharWidth() == 0 &&
+ "Unexpected non-bitfield with non-byte-aligned offset");
Expr.push_back(llvm::dwarf::DW_OP_plus_uconst);
Expr.push_back(
CGM.getContext().toCharUnitsFromBits(fieldOffset).getQuantity());
diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h
index 7b60e94555d060..af58ccdb5b35d2 100644
--- a/clang/lib/CodeGen/CGDebugInfo.h
+++
[clang] [llvm] [DebugInfo] Use DW_op_bit_piece for structured bindings of bitfields (PR #85665)
https://github.com/john-brawn-arm created
https://github.com/llvm/llvm-project/pull/85665
Currently we use DW_OP_plus_uconst to handle the bitfield offset and handle the
bitfield size by choosing a type size that matches, but this doesn't work if
either offset or size aren't byte-aligned. Using DW_op_bit_piece means we can
handle any offset and size.
>From 665d4034a1428d9b5cf1c4d4e89a16fa00b94fb5 Mon Sep 17 00:00:00 2001
From: John Brawn
Date: Thu, 14 Mar 2024 16:17:03 +
Subject: [PATCH] [DebugInfo] Use DW_op_bit_piece for structured bindings of
bitfields
Currently we use DW_OP_plus_uconst to handle the bitfield offset and
handle the bitfield size by choosing a type size that matches, but
this doesn't work if either offset or size aren't byte-aligned. Using
DW_op_bit_piece means we can handle any offset and size.
---
clang/lib/CodeGen/CGDebugInfo.cpp | 52 ++--
clang/lib/CodeGen/CGDebugInfo.h | 3 -
...debug-info-structured-binding-bitfield.cpp | 118 +-
.../CodeGen/AsmPrinter/DwarfExpression.cpp| 3 +
llvm/lib/IR/DebugInfoMetadata.cpp | 2 +
5 files changed, 70 insertions(+), 108 deletions(-)
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp
b/clang/lib/CodeGen/CGDebugInfo.cpp
index c2c01439f2dc99..bf142bdbbe0f99 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4777,40 +4777,6 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const
VarDecl *VD,
return D;
}
-llvm::DIType *CGDebugInfo::CreateBindingDeclType(const BindingDecl *BD) {
- llvm::DIFile *Unit = getOrCreateFile(BD->getLocation());
-
- // If the declaration is bound to a bitfield struct field, its type may have
a
- // size that is different from its deduced declaration type's.
- if (const MemberExpr *ME = dyn_cast(BD->getBinding())) {
-if (const FieldDecl *FD = dyn_cast(ME->getMemberDecl())) {
- if (FD->isBitField()) {
-ASTContext = CGM.getContext();
-const CGRecordLayout =
-CGM.getTypes().getCGRecordLayout(FD->getParent());
-const CGBitFieldInfo = RL.getBitFieldInfo(FD);
-
-// Find an integer type with the same bitwidth as the bitfield size. If
-// no suitable type is present in the target, give up on producing
debug
-// information as it would be wrong. It is certainly possible to
produce
-// correct debug info, but the logic isn't currently implemented.
-uint64_t BitfieldSizeInBits = Info.Size;
-QualType IntTy =
-Context.getIntTypeForBitwidth(BitfieldSizeInBits, Info.IsSigned);
-if (IntTy.isNull())
- return nullptr;
-Qualifiers Quals = BD->getType().getQualifiers();
-QualType FinalTy = Context.getQualifiedType(IntTy, Quals);
-llvm::DIType *Ty = getOrCreateType(FinalTy, Unit);
-assert(Ty);
-return Ty;
- }
-}
- }
-
- return getOrCreateType(BD->getType(), Unit);
-}
-
llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const BindingDecl *BD,
llvm::Value *Storage,
std::optional ArgNo,
@@ -4825,7 +4791,8 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const
BindingDecl *BD,
if (isa(BD->getBinding()))
return nullptr;
- llvm::DIType *Ty = CreateBindingDeclType(BD);
+ llvm::DIFile *Unit = getOrCreateFile(BD->getLocation());
+ llvm::DIType *Ty = getOrCreateType(BD->getType(), Unit);
// If there is no debug info for this type then do not emit debug info
// for this variable.
@@ -4851,7 +4818,6 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const
BindingDecl *BD,
unsigned Column = getColumnNumber(BD->getLocation());
StringRef Name = BD->getName();
auto *Scope = cast(LexicalBlockStack.back());
- llvm::DIFile *Unit = getOrCreateFile(BD->getLocation());
// Create the descriptor for the variable.
llvm::DILocalVariable *D = DBuilder.createAutoVariable(
Scope, Name, Unit, Line, Ty, CGM.getLangOpts().Optimize,
@@ -4865,13 +4831,13 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const
BindingDecl *BD,
const ASTRecordLayout =
CGM.getContext().getASTRecordLayout(parent);
const uint64_t fieldOffset = layout.getFieldOffset(fieldIndex);
-
- if (fieldOffset != 0) {
-// Currently if the field offset is not a multiple of byte, the
produced
-// location would not be accurate. Therefore give up.
-if (fieldOffset % CGM.getContext().getCharWidth() != 0)
- return nullptr;
-
+ if (FD->isBitField()) {
+Expr.push_back(llvm::dwarf::DW_OP_bit_piece);
+Expr.push_back(FD->getBitWidthValue(CGM.getContext()));
+Expr.push_back(fieldOffset);
+ } else if (fieldOffset != 0) {
+assert(fieldOffset % CGM.getContext().getCharWidth() == 0 &&
+ "Unexpected non-bitfield with non-byte-aligned offset");
