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@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
