[llvm] r312165 - Refactor DIBuilder::createFragmentExpression into a static DIExpression member

[Francois Pichet via llvm-commits]

On Mon, Oct 30, 2017 at 5:39 PM, Adrian Prantl <aprantl at apple.com> wrote:

>
>
> On Oct 30, 2017, at 2:25 PM, Francois Pichet <pichet2000 at gmail.com> wrote:
>
>
>
> On Mon, Oct 30, 2017 at 5:15 PM, Adrian Prantl <aprantl at apple.com> wrote:
>
>>
>>
>> On Oct 30, 2017, at 2:12 PM, Francois Pichet <pichet2000 at gmail.com>
>> wrote:
>>
>>
>>
>> On Wed, Aug 30, 2017 at 4:04 PM, Adrian Prantl via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>> Author: adrian
>>> Date: Wed Aug 30 13:04:17 2017
>>> New Revision: 312165
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=312165&view=rev
>>> Log:
>>> Refactor DIBuilder::createFragmentExpression into a static DIExpression
>>> member
>>>
>>> NFC
>>>
>>> Modified:
>>>     llvm/trunk/include/llvm/IR/DIBuilder.h
>>>     llvm/trunk/include/llvm/IR/DebugInfoMetadata.h
>>>     llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp
>>>     llvm/trunk/lib/IR/DIBuilder.cpp
>>>     llvm/trunk/lib/IR/DebugInfoMetadata.cpp
>>>     llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp
>>>     llvm/trunk/lib/Transforms/Scalar/SROA.cpp
>>>
>>>
>>>
>>> Modified: llvm/trunk/lib/IR/DebugInfoMetadata.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/D
>>> ebugInfoMetadata.cpp?rev=312165&r1=312164&r2=312165&view=diff
>>> ============================================================
>>> ==================
>>> --- llvm/trunk/lib/IR/DebugInfoMetadata.cpp (original)
>>> +++ llvm/trunk/lib/IR/DebugInfoMetadata.cpp Wed Aug 30 13:04:17 2017
>>> @@ -724,6 +724,34 @@ DIExpression *DIExpression::prepend(cons
>>>    return DIExpression::get(Expr->getContext(), Ops);
>>>  }
>>>
>>> +DIExpression *DIExpression::createFragmentExpression(const
>>> DIExpression *Expr,
>>> +                                                     unsigned
>>> OffsetInBits,
>>> +                                                     unsigned
>>> SizeInBits) {
>>> +  SmallVector<uint64_t, 8> Ops;
>>> +  // Copy over the expression, but leave off any trailing
>>> DW_OP_LLVM_fragment.
>>> +  if (Expr) {
>>> +    for (auto Op : Expr->expr_ops()) {
>>> +      if (Op.getOp() == dwarf::DW_OP_LLVM_fragment) {
>>> +        // Make the new offset point into the existing fragment.
>>> +        uint64_t FragmentOffsetInBits = Op.getArg(0);
>>> +        // Op.getArg(0) is FragmentOffsetInBits.
>>> +        // Op.getArg(1) is FragmentSizeInBits.
>>> +        assert((OffsetInBits + SizeInBits <= Op.getArg(0) +
>>> Op.getArg(1)) &&
>>> +               "new fragment outside of original fragment");
>>>
>>
>>
>>  Hi,
>>
>> I am getting an assert here on an (out of tree) big-endian target where
>> 64-bit variables are splitted on 2 32-bit variables.
>> assert where: OffsetInBits=32, SizeInBits=32 and
>> Expr: !DIExpression(DW_OP_LLVM_fragment, 0, 32)
>> so 32 + 32 <= 32 is not true.
>>
>> I might be wrong but I think the assert is only valid on little endian
>> target?
>>
>>
>> I'm not sure I understand your example. Are you saying that you are
>> calling createFragmentExpression on something that is already 32-bit
>> fragment? Which pass is calling it? It seems more likely that there is
>> something wrong with the call site.
>>
>> -- adrian
>>
>>
>> DAGTypeLegalizer::SetExpandedInteger is the call site:
>
> createFragmentExpression creates a fragment of and existing value at a
> given offset, and with a given size. If the existing value already is a
> fragment the offset and size are *within* the existing fragment. The assert
> verifies this. It should be endianness-neutral. Can you explain why you
> believe the assertion is wrong for big-endian targets?
>
>
>   if (DAG.getDataLayout().isBigEndian()) {
>     transferDbgValues(DAG, Op, Hi, 0);
>     transferDbgValues(DAG, Op, Lo, Hi.getValueSizeInBits());  // <---
> assert
>
>
> This code was added in https://reviews.llvm.org/D38172. This is splitting
> the debug info into one part describing the Hi bits (at offset 0) and the
> Lo bits (at sizeof(Hi)). Since you are seeing the assertion I assume that
> you have a value that is split twice, is that correct? Can you explain why
> createFragmentExpression is called with a !DIExpression(DW_OP_LLVM_fragment,
> 0, 32)?
>
> thanks,
>
>
Hi,

The problem happen when Op = ISD::build_pair and Lo and Hi already have
fragment created by SROA.cpp.

More detail here:
https://gist.github.com/anonymous/5d72495c77f28269fe3c01715e770e19

Not sure what the solution is.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171031/eedcb043/attachment.html>