[PATCH] D48331: [WIP][DebugInfo][InstCombine] Preserve DI after merging zext instructions

[Anastasis via Phabricator via llvm-commits]

gramanas added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1094
+            if (DIExp) {
+              return DIExpression::createFragmentExpression(
+                         DIExp, DestBitSize - SrcBitsKept,
----------------
aprantl wrote:
> gramanas wrote:
> > vsk wrote:
> > > You mentioned offline that this causes a verifier failure. If we can't use fragments, consider using a simpler approach for scalars: using an empty expression. It won't work for vectors, but in the scalar case, the high bits should already be cleared. We can relax the check in https://reviews.llvm.org/D48408 to allow this.
> > If I use an empty DIExpression it doesn't change the fact that the dbg.value call points to a DILocalVariable that after instcombine has a different DIBasicType than what it used to.
> > 
> > Basiacly the dbg.value call says that the value was changed to an i32 but the it's type is still i8 in the debug info. If there is a way to explain this to the debugger using DIExpressions, I can't find it.
> (Not sure if that is your question) We don't have any means to express that the SSA value used by a dbg.value instrinsic is larger (e.g., because of s/zext) than the size of the fragment being described. 
> 
> For example, if the size of the variable is 8 bits and the value is 32 bits then implicitly the lower 8 bits are used. You could make this explicit by masking out everything but the lower out bits in the DIExpression (`DW_OP_constu 0xff DW_OP_and`) but this is redundant so we don't do it.
> 
> Why would you like to express this?
I thought the fact that a 32 bit variable has DIBasicType of size 8 is wrong. But modifying the DI would interfere with the way the front-end described the program at the first place, thus I was thinking of a way to describe it with a DIExpression. Since the correct value is being displayed implicitly, I guess it's not a problem.


Repository:
  rL LLVM

https://reviews.llvm.org/D48331