[PATCH] D11933: Extend debug ranges and provide multiple location support for debug variables

[Vikram TV via llvm-commits]

tvvikram added a comment.

In http://reviews.llvm.org/D11933#222154, @aprantl wrote:

> I didn't have a chance to actually play with the code so far (and the patch didn't apply cleanly), but here are some very high level comments and questions:
>
> - Why did you choose to insert DBG_VALUEs into the machine function instead of implementing it directly inside DbgValueHistoryCalculator?


I have written a brief note about it at start of DebugValueFixup.cpp.

> - Having it as a separate pass could actually be good for testability — did you look into using the brand new MIR serialization format for writing MIR->MIR testcases?


I am quite new to MIR serialization. I will try.

> - There should also be a test case with an aggregate variable that is split across multiple DW_OP_pieces to make sure they are handled correctly.


Will try to come up with one.

> - Did you run any benchmarks with the new pass? Does it cause any measurable slowdown?


I have not tried any benchmarks as of yet.

> The MultipleLocations handling should probably be split out into a separate patch and reviewed separately.


Creating 2 new patches.

> thanks,

> adrian





http://reviews.llvm.org/D11933