On Mon, May 12, 2014 at 11:48 AM, Robinson, Paul <[email protected]> wrote: > Re. the cruelty of macros: I have a case in my backlog where > a macro expands to multiple statements; we emit the same source > location for each, but put a separate is_stmt flag on each > statement. Literally true but not helpful to the debugging > experience. > > Given that a debugger would be stepping through source as it > was originally provided, my feeling is that we should treat > the whole mass as a single megastatement whose source location > is the location of the macro invocation.
Doesn't sound wholely unreasonable - comparisons with GCC, some experience with debuggers and what they think of is_stmt, would be helpful if/when you/we/someone gets to that issue. - David > --paulr > >> -----Original Message----- >> From: [email protected] [mailto:cfe-commits- >> [email protected]] On Behalf Of David Blaikie >> Sent: Friday, May 09, 2014 7:45 PM >> To: [email protected] >> Subject: r208468 - Add FIXME describing the limitation of using column >> info to disambiguate inlining. >> >> Author: dblaikie >> Date: Fri May 9 21:44:57 2014 >> New Revision: 208468 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=208468&view=rev >> Log: >> Add FIXME describing the limitation of using column info to disambiguate >> inlining. >> >> Also tidy up, simplify, and extend the test coverage to demonstrate the >> limitations. This test should now fail if the bugs are fixed (& >> hopefully whoever ends up in this situation sees the FIXMEs and realizes >> that the test needs to be updated to positively test their change that >> has fixed some or all of these issues). >> >> I do wonder whether I could demonstrate breakage without a macro here, >> but any way I slice it I can't think of a way to get two calls to the >> same function on the same line/column in non-macro C++ - implicit >> conversions happen at the same location as an explicit function, but >> you'd never get an implicit conversion on the result of an explicit call >> to the same implicit conversion operator (since the value is already >> converted to the desired result)... >> >> Modified: >> cfe/trunk/lib/CodeGen/CGExpr.cpp >> cfe/trunk/test/CodeGenCXX/debug-info-same-line.cpp >> >> Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp >> URL: http://llvm.org/viewvc/llvm- >> project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=208468&r1=208467&r2=208468& >> view=diff >> ======================================================================== >> ====== >> --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original) >> +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Fri May 9 21:44:57 2014 >> @@ -2862,6 +2862,10 @@ RValue CodeGenFunction::EmitCallExpr(con >> SourceLocation Loc = E->getLocStart(); >> // Force column info to be generated so we can differentiate >> // multiple call sites on the same line in the debug info. >> + // FIXME: This is insufficient. Two calls coming from the same >> macro >> + // expansion will still get the same line/column and break debug >> info. It's >> + // possible that LLVM can be fixed to not rely on this uniqueness, >> at which >> + // point this workaround can be removed. >> const FunctionDecl* Callee = E->getDirectCallee(); >> bool ForceColumnInfo = Callee && Callee->isInlineSpecified(); >> DI->EmitLocation(Builder, Loc, ForceColumnInfo); >> @@ -3131,6 +3135,10 @@ RValue CodeGenFunction::EmitCall(QualTyp >> >> // Force column info to differentiate multiple inlined call sites on >> // the same line, analoguous to EmitCallExpr. >> + // FIXME: This is insufficient. Two calls coming from the same macro >> expansion >> + // will still get the same line/column and break debug info. It's >> possible >> + // that LLVM can be fixed to not rely on this uniqueness, at which >> point this >> + // workaround can be removed. >> bool ForceColumnInfo = false; >> if (const FunctionDecl* FD = dyn_cast_or_null<const >> FunctionDecl>(TargetDecl)) >> ForceColumnInfo = FD->isInlineSpecified(); >> >> Modified: cfe/trunk/test/CodeGenCXX/debug-info-same-line.cpp >> URL: http://llvm.org/viewvc/llvm- >> project/cfe/trunk/test/CodeGenCXX/debug-info-same- >> line.cpp?rev=208468&r1=208467&r2=208468&view=diff >> ======================================================================== >> ====== >> --- cfe/trunk/test/CodeGenCXX/debug-info-same-line.cpp (original) >> +++ cfe/trunk/test/CodeGenCXX/debug-info-same-line.cpp Fri May 9 >> 21:44:57 2014 >> @@ -6,118 +6,59 @@ >> >> #define INLINE inline __attribute__((always_inline)) >> >> -INLINE int >> -product (int x, int y) >> -{ >> - int result = x * y; >> - return result; >> -} >> - >> -INLINE int >> -sum (int a, int b) >> -{ >> - int result = a + b; >> - return result; >> -} >> +int i; >> >> -int >> -strange_max (int m, int n) >> -{ >> - if (m > n) >> - return m; >> - else if (n > m) >> - return n; >> - else >> - return 0; >> +INLINE void sum(int a, int b) { >> + i = a + b; >> } >> >> -int >> -foo (int i, int j) >> -{ >> - if (strange_max (i, j) == i) >> - return product (i, j); >> - else if (strange_max (i, j) == j) >> - return sum (i, j); >> - else >> - return product (sum (i, i), sum (j, j)); >> +void noinline(int x, int y) { >> + i = x + y; >> } >> >> -int >> -main(int argc, char const *argv[]) >> -{ >> +#define CALLS sum(9, 10), sum(11, 12) >> >> - int array[3]; >> - int n; >> - >> - array[0] = foo (1238, 78392); >> - array[1] = foo (379265, 23674); >> - array[2] = foo (872934, 234); >> - >> - n = strange_max(array[0], strange_max(array[1], array[2])); >> +inline void inlsum(int t, int u) { >> + i = t + u; >> +} >> >> - return n & 0xf; >> +int main() { >> + sum(1, 2), sum(3, 4); >> + noinline(5, 6), noinline(7, 8); >> + CALLS; >> + inlsum(13, 14), inlsum(15, 16); >> } >> >> -// CHECK: define {{.*}} @_Z3fooii >> -// i >> -// CHECK: call void @llvm.dbg.declare >> -// j >> -// CHECK: call void @llvm.dbg.declare >> -// x >> -// CHECK: call void @llvm.dbg.declare >> -// y >> -// CHECK: call void @llvm.dbg.declare >> -// result >> -// CHECK: call void @llvm.dbg.declare >> - >> -// CHECK: call void @llvm.dbg.declare(metadata !{i32* %{{.*}}}, >> metadata ![[A_MD:[0-9]+]]), !dbg ![[A_DI:[0-9]+]] >> -// CHECK: call void @llvm.dbg.declare(metadata !{i32* %{{.*}}}, >> metadata ![[B_MD:[0-9]+]]), !dbg ![[B_DI:[0-9]+]] >> -// result >> -// CHECK: call void @llvm.dbg.declare >> - >> -// We want to see a distinct !dbg node. >> -// CHECK-NOT: call void @llvm.dbg.declare(metadata !{i32* %{{.*}}}, >> metadata ![[A_MD]]), !dbg ![[A_DI]] >> -// CHECK: call void @llvm.dbg.declare(metadata !{i32* %{{.*}}}, >> metadata ![[A_MD]]), !dbg !{{.*}} >> -// CHECK-NOT: call void @llvm.dbg.declare(metadata !{i32* %{{.*}}}, >> metadata ![[B_MD]]), !dbg ![[B_DI]] >> -// CHECK: call void @llvm.dbg.declare(metadata !{i32* %{{.*}}}, >> metadata ![[B_MD]]), !dbg !{{.*}} >> -// result >> -// CHECK: call void @llvm.dbg.declare >> - >> -// We want to see a distinct !dbg node. >> -// CHECK-NOT: call void @llvm.dbg.declare(metadata !{i32* %{{.*}}}, >> metadata ![[A_MD]]), !dbg ![[A_DI]] >> -// CHECK: call void @llvm.dbg.declare(metadata !{i32* %{{.*}}}, >> metadata ![[A_MD]]), !dbg !{{.*}} >> -// CHECK-NOT: call void @llvm.dbg.declare(metadata !{i32* %{{.*}}}, >> metadata ![[B_MD]]), !dbg ![[B_DI]] >> -// CHECK: call void @llvm.dbg.declare(metadata !{i32* %{{.*}}}, >> metadata ![[B_MD]]), !dbg !{{.*}} >> -// result >> -// CHECK: call void @llvm.dbg.declare >> - >> -// Again: we want to see a distinct !dbg node. >> -// CHECK: call void @llvm.dbg.declare(metadata !{i32* %{{.*}}}, >> metadata ![[X_MD:[0-9]+]]), !dbg ![[X_DI:[0-9]+]] >> -// CHECK: call void @llvm.dbg.declare(metadata !{i32* %{{.*}}}, >> metadata ![[Y_MD:[0-9]+]]), !dbg ![[Y_DI:[0-9]+]] >> -// result >> -// CHECK: call void @llvm.dbg.declare >> - >> - >> -// CHECK: define {{.*}} @main >> -// CHECK: call {{.*}} @_Z3fooii >> -// CHECK: call {{.*}} @_Z3fooii >> -// CHECK: call {{.*}} @_Z3fooii >> -// CHECK: store >> -// CHECK: getelementptr >> -// We want to see the same !dbg node for non-inlined functions. >> -// Needed for GDB compatibility. >> -// CHECK: load {{.*}} !dbg ![[DBG:.*]] >> -// CHECK: load {{.*}} !dbg ![[DBG]] >> -// CHECK: load {{.*}} !dbg ![[DBG]] >> -// CHECK: call {{.*}} @_Z11strange_maxii(i32 {{.*}}, i32 {{.*}}), !dbg >> ![[DBG]] >> -// CHECK: call {{.*}} @_Z11strange_maxii(i32 {{.*}}, i32 {{.*}}), !dbg >> ![[DBG]] >> - >> - >> -// Verify that product() has its own inlined_at location at column 15. >> -// CHECK-DAG: ![[A_MD]] = metadata{{.*}}[ DW_TAG_arg_variable ] [a] >> -// CHECK-DAG: ![[B_MD]] = metadata{{.*}}[ DW_TAG_arg_variable ] [b] >> -// CHECK-DAG: ![[X_MD]] = metadata{{.*}}[ DW_TAG_arg_variable ] [x] >> -// CHECK-DAG: ![[Y_MD]] = metadata{{.*}}[ DW_TAG_arg_variable ] [y] >> -// CHECK-DAG: ![[X_DI]] = metadata !{i32 {{[0-9]+}}, i32 {{[0-9]+}}, >> metadata !{{[0-9]+}}, metadata ![[PRODUCT:[0-9]+]]} >> -// CHECK-DAG: [[PRODUCT]] = metadata !{i32 {{.*}}, i32 16, metadata >> !{{.*}}, null} >> -// CHECK-DAG: ![[Y_DI]] = metadata !{i32 {{[0-9]+}}, i32 {{[0-9]+}}, >> metadata !{{[0-9]+}}, metadata ![[PRODUCT]]} >> +// CHECK-LABEL: @main >> +// CHECK: = add {{.*}} !dbg [[FIRST_INLINE:![0-9]*]] >> +// CHECK: = add {{.*}} !dbg [[SECOND_INLINE:![0-9]*]] >> + >> +// Check that we don't give column information (and thus end up with >> distinct >> +// line entries) for two non-inlined calls on the same line. >> +// CHECK: call {{.*}}noinline{{.*}}(i32 5, i32 6), !dbg [[NOINLINE:![0- >> 9]*]] >> +// CHECK: call {{.*}}noinline{{.*}}(i32 7, i32 8), !dbg [[NOINLINE]] >> + >> +// FIXME: These should be separate locations but because the two calls >> have the >> +// same line /and/ column, they get coalesced into a single inlined >> call by >> +// accident. We need discriminators or some other changes to LLVM to >> cope with >> +// this. (this is, unfortunately, an LLVM test disguised as a Clang >> test - since >> +// inlining is forced to happen here). It's possible this could be >> fixed in >> +// Clang, but I doubt it'll be the right place for the fix. >> +// CHECK: = add {{.*}} !dbg [[FIRST_MACRO_INLINE:![0-9]*]] >> +// CHECK: = add {{.*}} !dbg [[FIRST_MACRO_INLINE]] >> + >> +// Even if the functions are marked inline but do not get inlined, they >> +// shouldn't use column information, and thus should be at the same >> debug >> +// location. >> +// CHECK: call {{.*}}inlsum{{.*}}(i32 13, i32 14), !dbg >> [[INL_FIRST:![0-9]*]] >> +// CHECK: call {{.*}}inlsum{{.*}}(i32 15, i32 16), !dbg >> [[INL_SECOND:![0-9]*]] >> + >> +// [[FIRST_INLINE]] = >> +// [[SECOND_INLINE]] = >> + >> +// FIXME: These should be the same location since the functions appear >> on the >> +// same line and were not inlined - they needlessly have column >> information >> +// intended to disambiguate inlined calls, which is going to confuse >> GDB as it >> +// doesn't cope well with column information. >> +// [[INL_FIRST]] = >> +// [[INL_SECOND]] = >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
