[PATCH] D73652: Defer composing subregisters with DW_OP_piece

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added a comment.

In D73652#1849033 <https://reviews.llvm.org/D73652#1849033>, @bjope wrote:

> Thanks for the NFC split!
>  I was a little bit confused at first (about "Diff 241236") but it just seems like you have uploaded the reverse diff (so the new code is to the left and the old code is to the right when looking at the diff in Phabricator).


Nice, so I screwed it up twice m-(

I tried to get the previous diff by doing `git diff -U999 HEAD~1..HEAD~2` and got the order wrong.

> One worry is that for input like this
> 
>   DBG_VALUE $reg128bits, $noreg, !7, !DIExpression(DW_OP_LLVM_fragment, 0, 32)
> 
> 
> where reg128bits would be the composition of four 32-bit registers on some imaginary target,
>  then we used to set MaxSize=32 and then we could return true from addMachineReg after finding the first subregister (covering the least significant bits) of reg128bits.
>  Now we will return false if any of the upper bits can't be described.

Agreed. Is that a hypothetical, or should we address this because such targets might actually exist?

> This change is of course needed, considering that we want to handle things like
> 
>   DBG_VALUE $reg128bits, $noreg, !7, !DIExpression(DW_OP_constu 96, DW_OP_shr, DW_OP_LLVM_fragment, 0, 32)
> 
> 
> when the most significant bits of reg128bits are of interest. But maybe we want to re-introduce the MaxSize in the future, but being more careful when to restrict the MaxSize.

Right. It would only help in that exact edge case, but not with the complex expressions that nowadays fall out of salvageDebugInfo().

> One simple thing would be to set MaxSize when the DIExpression only contains a DW_OP_LLVM_fragment (i.e. we could still use MaxSize when `HasComplexExpression` is false).
>  In the long term (as a FIXME), we could analyse the DIExpression further to find out how many of the least significant bits in the register that are "demanded".




CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73652/new/

https://reviews.llvm.org/D73652