[PATCH] D41697: [DebugInfo][Metadata] Add support for a DIExpression as 'count' field of DISubrange.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a subscriber: huntergr.
sdesmalen added a comment.

In https://reviews.llvm.org/D41697#968546, @aprantl wrote:

> hard-coding a DW_OP_breg46 DW_OP_constu, 2, DW_OP_mul, DW_OP_stack_value in the frontend Note that you can't combine DW_OP_regX with other operators, you have to use a combination of DW_OP_bregX DW_OP_stack_value instead (cf. DWARF5 section 2.6.1ff).


Are you sure the DW_OP_stack_value really needed? I thought this was only needed when the expression is used as a location expression.

> You will have to extend the backend (DwarfExpression.cpp) and the Verifier to accept a DW_OP_(b)reg inside a DIExpression.

You're right, I forgot that I added that support downstream a while back, I'll see if I can share a separate patch for that as well!

> Use an approach similar to llvm.dbg.value to bind the register and the DIExpression. But since Reg46 isn't actually used in the program, you would have to, e.g., create an intrinsic that returns Reg46, which is probably a lot more work for no clear benefit.

The SVE types used in the front-end come from our language extensions (ACLE), so we actually have an intrinsic available that we could optimize into a 'reg46' dwarf expression by matching the intrinsic, but then we'd still need to emit some target-specific intrinsic. An alternative would be to use a generic intrinsic for that... I know @huntergr has some patches downstream for a generic 'scalable vector width' intrinsic (https://reviews.llvm.org/D32530#912649) that we could use in the future.

> The interesting question here is again how to best bind the location to the rest of the DIExpression. For regular variables we use the dbg.value intrinsic to do this:
> 
>   %ptr = ...  ; in our example, a pointer to the variable
>   call @llvm.dbg.value(metadata %ptr, metadata !DIVariable(name: "x", ...), metadata !DIExpression(DW_OP_deref))
>    
> 
> This way metadata never points back to IR, and DIExpressions can be shared by multiple intrinsics.
> 
> As you proposed, we can create a pseudo-variable to inject a location into a DIRange:
> 
>   %length = load i32 %array...
>   call @llvm.dbg.value(metadata %length, metadata !1, metadata !DIExpression())
>   ...
>   !1 = DIVariable(name: "$count", ...)
>   !2 = !DIRange(count: metadata !1)
>    
> 
> A couple of questions now pop up:
> 
> - It looks like we are dropping the DIExpression because we link the DIVariable to the DIRange, not the dbg.value. Note that many LLVM optimizations augment DIExpressions.

I'm wondering why you're saying that the DIExpression is dropped? If we create a pseudo-variable like above, and the DIExpression is not empty, will it not just create a location list with the added expression to each location expression in the list? We can then reference the variable (with its own location list) from our count field?

> This doesn't solve the problem of how to combine two variables in one expression, but to me it looks like this question can be resolved independently by providing a mechanism for doing so in llvm.dbg.value.

Agreed!

> So long story short, I think your proposal is good.

Good to hear, thanks for digging into this and helping me understand the right way forward!


https://reviews.llvm.org/D41697