[patch] PR15662: reordered function parameters when inlining

Eric Christopher echristo at gmail.com
Tue Jun 4 16:46:13 PDT 2013


I think this is fine in general, couple of comments on the patch itself:

a) comment it with, basically, your commit message :)
b) please use 'I' for your iterator variable, having a lower case j as
an index and the lower case i as the iterator makes it more difficult
to see what's what IMO

Still a shame about DW_TAG_arg_variable, but it's to be put up with at
the moment.

Thanks!

-eric

On Tue, Jun 4, 2013 at 4:36 PM, David Blaikie <dblaikie at gmail.com> wrote:
> This fixes PR15662 as follows:
>
>     PR15662: Optimized debug info produces out of order function parameters
>
>     When a function is inlined we lazily construct the variables
>     representing the function's parameters. After that, we add any remaining
>     unused parameters.
>
>     If the function doesn't use all the parameters, or uses them out of
>     order, then the DWARF would produce them in that order, producing a
>     parameter order that doesn't match the source.
>
>     This fix causes us to always keep the arg variables at the start of the
>     variable list & in the original order from the source.
>
> The only issue is that this slows down slightly the non-opt case (we'd
> bail out when we find the variables list attached to the subprogram is
> empty) & moreso in the opt case (since we have to do the
> search/ordering work). We could do a bit more to make the loop at
> DwarfDebug.cpp:1679 be more efficient (rather than calling
> addScopeVariable each time causing a new search for the insertion
> point, we could factor it out to find/use the insertion point
> incrementally). I also didn't actually try to find a test case where
> variables were used in the inline function, but out of order. If such
> a case doesn't exist, then there would be no need to handle insertion
> order in the lazy/inlining case - just at the "put the rest in" at the
> end (the aforementioned loop). As it is, we could still make the lazy
> case better by continuing to be lazy, then re-ordering them just once
> at the end.
>
> Open to suggestions/preferences/ideas.



More information about the llvm-commits mailing list