================
@@ -555,8 +558,30 @@ bool DwarfExpression::addExpression(
// and not any other parts of the following DWARF expression.
assert(!IsEmittingEntryValue && "Can't emit entry value around expression");
- std::optional<DIExpression::ConvertOp> PrevConvertOp;
+ struct LabelOffset {
+ uint64_t ID;
+ uint64_t Offset;
+ };
+ struct BranchFixup {
+ uint64_t LabelID;
+ uint64_t PlaceholderOffset;
+ };
+ constexpr unsigned BranchOffsetByteSize = 2;
+
+ // Iterating over ExprCursor doesn't consume it.
+ bool HasSymbolicBranches =
+ llvm::any_of(ExprCursor, [](DIExpression::ExprOperand Op) {
+ return Op.is(dwarf::DW_OP_LLVM_bra) || Op.is(dwarf::DW_OP_LLVM_skip);
+ });
+
+ SmallVector<LabelOffset, 4> Labels;
+ SmallVector<BranchFixup, 4> Fixups;
+ // Buffer the expression until every label has a byte offset, then patch the
+ // branches.
+ if (HasSymbolicBranches)
----------------
slinder1 wrote:
The fact that `entry_value` only works in very narrow cases that this patch
avoids conflicting with is just a quirk of the current implementation. My
concern here is that re-using the same buffer adds a barrier to improving
`entry_value` handling in the future.
For example, if we just slightly relax `entry_value` to allow it to refer to
`arg 0` in expressions with only one arg (this is actually what I thought we
already supported, but it turns out we require `isSingleLocationExpression`),
then we should allow:
```
DW_OP_LLVM_label 0, DW_OP_LLVM_arg 0, DW_OP_LLVM_entry_value 1, DW_OP_bra 0
```
I imagine we could throw together supporting this in a similar way to what you
have here: we lookahead for the `entry_value` at the start of `addExpression`
and note where we need to do the buffering.
Without symbolic branches re-using the (single) buffer that is already used by
`entry_value` today this is a much simpler change.
We can just cross that road when we come to it, though; the `entry_value`
implementation seems like it should just be replaced with something that names
the register directly, and when lowering it we can generate the `entry_value`
block stuff. In the original proposal that eventually became the
locations-on-the-stack changes to DWARF6 we actually [added
`DW_OP_LLVM_call_frame_entry_reg
<REGNUM>`](https://llvm.org/docs/AMDGPUDwarfExtensionsForHeterogeneousDebugging.html#a-2-5-4-4-1-general-location-description-operations)
that does this at the DWARF level, but we can just lower an equivalent
LLVM-internal opcode to `entry_value` and still get the benefit in the compiler.
https://github.com/llvm/llvm-project/pull/210850
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
