[PATCH] D35994: Debug info for variables whos type is shrinked to bool

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added a comment.

In https://reviews.llvm.org/D35994#838113, @NikolaPrica wrote:

> This patch implements new DIExpression :
>
>   val * (ValOther - ValInit) + ValInit:


If we are doing this expression, there should be an assertion in the code that `ValInit < ValOther`.

>    DW_OP_deref DW_OP_constu <ValMinus>
>   DW_OP_mul DW_OP_constu <ValInit> DW_OP_plus DW_OP_stack_value

Why does `Val` need to be dereferenced?

If we use our concrete example with 42 and 87 again:

  start: <val> // either 0 or 1
  DW_OP_deref // not sure why this is needed, but let's assume we still have 0 or 1 on the stack
  DW_OP_constu (87-42) // <0 or 1> <45>
  DW_OP_mul // <0 or 45>
  DW_OP_constu 42 // <0 or 45> <42>
  DW_OP_plus // <42 or 87>
  DW_OP_stack_value

Apart from the DW_OP_deref, this seems very good, I'm slightly worried about the assumption that the init value will always be the smaller one.
What happens if any of the two values is negative (because of the typeless DWARF <5 expression stack — not sure if this will be a problem).

> The other expression which was suggested was :
> 
>   // val * 42 | ((~val)&1) * 87
>   DW_OP_dup DW_OP_deref DW_OP_constu 42 DW_OP_mul DW_OP_swap
>   DW_OP_deref DW_OP_not DW_OP_lit1 DW_OP_and DW_OP_constu 87 DW_OP_mul
>   DW_OP_or DW_OP_stack_value
>    
> 
> I have chosen to implement first because it is simpler.
>  I have also added simpler test also with concrete DW_AT_location description .



================
Comment at: lib/Transforms/IPO/GlobalOpt.cpp:1644
+  } else {
+    // FIXME: This will only emmit address for debugger on which will
+    // be written only 0 or 1.
----------------
typo: emit


https://reviews.llvm.org/D35994