Page MenuHomePhabricator

[DebugInfo] Introduce a DW_OP_LLVM_convert_generic operation
AcceptedPublic

Authored by dstenb on Mar 13 2020, 10:27 AM.

Details

Summary

DW_OP_LLVM_convert is used to do sign- and zero extensions in
DIExpressions. When using DWARFv5, that LLVM operation is translated
into DWARFv5's DW_OP_convert operation.

DWARFv5 section 2.5.1.4 specifies the following for arithmetical and
logical operations:

"Operands of an operation with two operands must have the same type,
either the same base type or the generic type."

So when we have used DW_OP_LLVM_convert in an expression, we need to
make sure that any arithmetical and logical operations operating on that
element have operands of the same type. As we currently don't have a
good interface for detecting what type is on top of the stack, and we
add arithmetical and logical operations, e.g. appending expressions like
{DW_OP_constu 1, DW_OP_plus}, in many places, this patch attempts to
alleviate such cases by introduced a new operation,
DW_OP_LLVM_convert_generic, which converts the top of the stack back to
the generic type.

At least initially, I think we should use this operation after each
sign- and zero extension done using DW_OP_LLVM_convert, until we are
more type aware on the stack. That is done in a follow-up patch.

I added a new operation rather than making DW_OP_LLVM_convert having a
special mode that converts to the generic type, e.g. when the size
operand is 0, as I thought this would be clearer.

Diff Detail

Event Timeline

dstenb created this revision.Mar 13 2020, 10:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 13 2020, 10:27 AM
djtodoro added inline comments.
llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
487

//<comment instead?

Have you thought about doing this

llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
487

I don't think that's a thing for non-doxygen comments.

The proper, if verbose, LLVM style is

/ ULEB128.
emitUnsigned(0);
vsk accepted this revision.Mar 16 2020, 4:37 PM

Thanks, lgtm.

This revision is now accepted and ready to land.Mar 16 2020, 4:37 PM

Have you thought about doing this

Thanks for the suggestions. As emitUnsigned() emit ULEB128 encoded data maybe I should just remove that comment.

dstenb updated this revision to Diff 251325.Mar 19 2020, 3:36 AM

Rebase on top of D76423. Add a test that verifies that the operation is emitted correctly in location lists. Without D76423 this patch would simply emit a DW_OP_convert with the first base type in ExprRefedBaseTypes, rather than 0x0.