[PATCH] D158185: [DebugInfo] Process single-location debug values in variadic form when producing DWARF

[Felipe de Azevedo Piovezan via Phabricator via llvm-commits]

fdeazeve added inline comments.


================
Comment at: llvm/lib/IR/DebugInfoMetadata.cpp:1347
 }
 bool DIExpression::isEntryValue() const {
+  auto singleLocElts = getSingleLocationExpressionElements();
----------------
StephenTozer wrote:
> fdeazeve wrote:
> > Orlando wrote:
> > > Can we have DIExpressions where there are two entry_value ops (applied to two `DW_OP_LLVM_arg`s)?
> > > 
> > > Perhaps that's a separate issue or maybe there is special case `isEntryValue`-equivalent code somewhere for variaidics?
> > This is a tricky one, as I don't think any of the "isX" or "startsWithX" methods were designed with variadics in mind. These methods should only be called when we know we have a single expression?
> > 
> > I wasn't around when the variadic design was made, but my first thought is to think that instead of having variadic expressions it would be cleaner to have  multiple expressions, one per value argument. But there are probably a lot of implications I can't see right now!
> > my first thought is to think that instead of having variadic expressions it would be cleaner to have multiple expressions, one per value argument
> 
> The expression needs to calculate a single value - it's not being used in this case for fragments/DW_OP_piece expressions, but as a function to reproduce a variable's value from a set of input values. For example, if you try to translate `%z = add %x, %y` into a DIExpression, you can't do so using one expression for `%x` and one expression for `%y` - you just need a single expression for `%z` that takes `%x` and `%y` as arguments.
Ah, this makes perfect sense, thanks! Knew I was missing something fundamental here :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158185