rnk added a comment.

Based on what we learned in https://llvm.org/PR43530, I think we still want to 
use the location of the call site and not line zero. :(



================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1431
+        }
+        BI->setDebugLoc(TheCallDL);
+        continue;
----------------
aprantl wrote:
> I still think an artificial (line 0) location would be less misleading for 
> debuggers, profilers, and optimization remarks.
That will cause problems for us in practice. There's discussion about this in 
D68747. Since that change, we treat line zero the same as "no location". If 
there are no locations in a basic block, then the whole block inherits the line 
number from the block layout predecessor, which could be unrelated. Keeping the 
inlined call site location gives us the highest likelihood that "step over" 
will stop at the next statement. Widely applying line zero to entire basic 
blocks will put us in that situation more often. We could certainly write a 
pass to backfill better source locations, but it seems preferable to not put 
ourselves in that position in the first place.

However, the effect you mention on profilers and optimization remarks is real 
and concerning. Users should have the power to work around it by removing the 
flag that applies this attribute, which makes me feel like we should go forward 
with this as is. If this develops into a real usability problem, we can leave 
the attribute as is and move the implementation into the backend.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67723



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to