[PATCH] D134429: [DebugInfo][LICM] Drop DebugLoc from IntrinsicInst when hoisting

[Juan Manuel Martinez Caamaño via Phabricator via llvm-commits]

jmmartinez added a comment.

In D134429#3816054 <https://reviews.llvm.org/D134429#3816054>, @dblaikie wrote:

> In D134429#3815561 <https://reviews.llvm.org/D134429#3815561>, @jmmartinez wrote:
>
>> In D134429#3815441 <https://reviews.llvm.org/D134429#3815441>, @dblaikie wrote:
>>
>>> What's the motivation for the change - reduced debug info size by having fewer zero locations?
>>
>> The idea is to reduce the amount zero locations since they can be confusing for the users.
>
> At least GDB mostly ignores line zero, I think - so what sort of user confusion are you encountering/trying to address? Good to know what the use cases are, etc.

The issue was raised, in two separate occasions, from people developing static-analysis tools that maps the assembly back to the source code (for example, to map register spills back to source-code).



================
Comment at: llvm/lib/IR/DebugInfo.cpp:827
   if (!isa<CallBase>(this)) {
     setDebugLoc(DebugLoc());
     return;
----------------
rnk wrote:
> We now have three instances of `setDebugLoc(DebugLoc())` in this function. I think the "early return" guidance is leading us to code duplication.
> 
> Can you please refactor this so that we calculate the conditions under which we want to apply a line zero location, and have that be the early return case? It is, if this is an intrinsic which may lower to a call, or a non-intrinsic function call.
Updated to avoid code duplication. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134429/new/

https://reviews.llvm.org/D134429