[PATCH] D11933: Extending debug ranges

[Vikram TV via llvm-commits]

tvvikram added a comment.

In http://reviews.llvm.org/D11933#222508, @tvvikram wrote:

> 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.


There were pieces-*.ll tests under test/DebugInfo/ that handled aggregate variables. I have corrected them to match the new ranges that get generated.

> 

> 

> > - 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