[patch] FastISel omitting unused parameters hurts debug info

[David Blaikie]

On Sat, Jun 8, 2013 at 12:53 PM, JF Bastien <jfb at google.com> wrote:
> Would it be possible to configure this behavior? i.e. to have "fast but not
> silly isel", and "fast and debuggable isel"?

We don't currently have a means to differentiate these things. There's
some discussion about adding a -Og option to be the latter. Not sure
if there are other proposals/ideas on addressing that issue.

> The later should probably be
> the default, but some people (however silly they may be) want fast code from
> fast isel. ;-)

& if those people care, they can address the fixme:

// FIXME: Unfortunately it's necessary to emit a copy from the livein copy.
// Without this, EmitLiveInCopies may eliminate the livein if its only
// use is a bitcast (which isn't turned into an instruction).

It seems that this should be fixed mostly for all the /used/
parameters its hurting. I assume that the number of unused parameters
is relatively small in comparison to not make things substantially
worse.

If/when that issue is fixed, my change here shouldn't affect the
generated code at all.

At some point later we'll want to care about keeping parameters live
for the life of the function, probably in a -Og-like mode, so users
could examine/manipulate the parameters for the life of the function
rather than until their last use (which, in the degenerate/extreme
case of unused parameters, is the end of the prologue).

- David

>
>
> On Fri, Jun 7, 2013 at 10:27 AM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>> [+Eric, since I forgot him on the first mail]
>>
>> One other avenue that I haven't explored yet is that, perhaps,
>> something like r182062 might be applicable, which simply retained more
>> argument frame index information without generating more machine
>> registers/instructions. I don't know at all if that applies to this
>> issue, but it did address a similar issue of debug info missing on
>> unused parameters (PR14575).
>>
>> On Thu, Jun 6, 2013 at 7:52 PM, David Blaikie <dblaikie at gmail.com> wrote:
>> > Hi Dan,
>> >
>> > Here's the patch we talked about this afternoon - with the equivalent
>> > change made to the ARM side of fast ISel to match the X86 change I'd
>> > made. Turns out with the ARM change something does fail -
>> > CodeGen/ARM/fast-isel-call.ll. So it seems this change isn't neutral
>> > to codegen. (though I'm not sure how much we need to care deeply about
>> > the performance of unused parameters?) I assume the source of the
>> > change is the FIXME immediately proceeding the change to *FastISel.cpp
>> > - so perhaps this is an acceptable "we'll fix the underlying issue &
>> > this and some other things will get better".
>> >
>> > I was wondering if you had any ideas on how this should be addressed
>> > (is this acceptable? in which case I should just update the test case
>> > (though I don't fully understand it yet). I've attached the two .s
>> > files, with my change (a.s) and without my change (b.s) (around
>> > r183465) along with the patch itself.
>> >
>> > - David
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>