[Lldb-commits] [PATCH] D91734: [FastISel] Flush local value map on every instruction

Jim Ingham via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 3 14:50:48 PST 2020


Note, lldb has a bunch of special handling for line 0 code.  For instance, when we are pushing a breakpoint past the prologue we will keep pushing it forward over line number 0 lines.  Those are compiler generated and in general people don't want to stop there.  Similarly, if you are stepping through line 3 and the next line entry after 3 is line 0 we keep stepping till we get to a non-zero line.  

When the compiler is actually using line 0 to mean "compiler generated code not really associated with a particular line, then I am pretty sure the debugger has to be aware of this or debugging is going to be a bit awkward...

I don't know if that's directly relevant to this bug, I haven't had time to follow the whole discussion.  But I'm not convinced all the problems with line 0 emission causing debugging oddities can be solved in the line table generation.

Jim


> On Dec 3, 2020, at 10:33 AM, David Blaikie via Phabricator via lldb-commits <lldb-commits at lists.llvm.org> wrote:
> 
> dblaikie added a comment.
> 
> In D91734#2431247 <https://reviews.llvm.org/D91734#2431247>, @probinson wrote:
> 
>>> Sometihng like this seems plausible to me:
>> 
>> Yes, I was playing with essentially that exact patch last night.  It has no effect on the final assembly on its own, but combined with my patch it does.
> 
> It might have effects on assembly in other test cases, though. Could be worth running it through a self-host or something to see what other changes it causes and whether they're desirable.
> 
>>> (a more general fix (that would cover cases where the instruction really has no location) might be to propagate locations from the first instruction in a basic block with a location back up to the start of the basic block - I forget if we've considered/tried that before)
>> 
>> We have, but that without flushing the map on every instruction, so it caught materialization instructions that didn't actually belong to that IR instruction.  The tactic would likely be more reasonable in conjunction with my patch.
> 
> (oh, when I was saying that I didn't really think - the materialization in this case wasn't necessarily on a BB boundary - so I guess my suggestion amounts to possibly never using line 0 (unless it's the only location in a whole BB), instead back or forward propagating surrounding locations over any line 0 - and that doesn't sound right when I put it that way)
> 
> But yeah, maybe some amount of it could be done around the flushing thing.
> 
> (FWIW, about this patch in general, I do worry a bit about this being a debug-info motivated optimization decision (even if that decision is applied uniformly/not just when debug info is enabled) - you mention this might have positive performance features due to smaller register live ranges, but also possibly negative ones rematerializing the same constant (" however, only about 5% of those values
> were reused in an experimental self-build of clang.") - do you have performance measurements/benchmarks related to this change? I guess it didn't show up in the perf bot profiles upstream at least)
> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D91734/new/
> 
> https://reviews.llvm.org/D91734
> 
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits



More information about the llvm-commits mailing list