krisb added a comment. @dblaikie, could you please take a look at this and/or D113741 <https://reviews.llvm.org/D113741>? Do you see any ways to proceed?
================ Comment at: llvm/test/Instrumentation/InstrProfiling/debug-info-correlate.ll:23 call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @__profn_foo, i32 0, i32 0), i64 12345678, i32 2, i32 0) - ret void + ret void, !dbg !17 } ---------------- ellis wrote: > krisb wrote: > > ellis wrote: > > > krisb wrote: > > > > ellis wrote: > > > > > I asked the same question in D113741, but why is this test changed? > > > > Normally, we emit function-local entities iff a parent function has > > > > location information. This is done the same way for local variables, > > > > labels, parameters, imported entities, and, //now,// for static locals > > > > as well. > > > > Before this change static locals behaved more like global variables and > > > > get emitted doesn't matter its parent function. This patch makes them > > > > handled more like local variables. > > > > > > > > I believe either the call or the 'ret' (or, likely, both) had had > > > > DILocation attached originally, but it has been removed to simplify the > > > > test. > > > I just checked and the `llvm.instrprof.increment` intrinsic does not have > > > debug info attached. I think you're right that I removed debug info from > > > the `ret` instruction to simplify the test. > > > > > > This is a special case where a global and its debug info is synthesized. > > > https://github.com/llvm/llvm-project/blob/23c2bedfd93cfacc62009425c464e659a34e92e6/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp#L976-L1001 > > > > > > So I don't understand why this added debug info is necessary. Does the > > > test fail otherwise? > > Right, the test would fail w/o the added DILocation, because `__profc_foo` > > variable (which is function-local 'global' technically) would not be > > emitted. > > With this patch we emit function-local entities if and only if the parent > > function has LexicalScope defined (see createAndAddScopeChildren() and its > > call from constructSubprogramScopeDIE()), otherwise we skip emitting all > > function's children (including function-local 'globals'). > Got it, thanks for explaining! > > I'm ok with this because `llvm.instrprof.increment` should probably be next > to some instruction with debug info. Thank you for checking this is okay! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125693/new/ https://reviews.llvm.org/D125693 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
