r208468 - Add FIXME describing the limitation of using column info to disambiguate inlining.
Robinson, Paul
Paul_Robinson at playstation.sony.com
Mon May 12 11:48:40 PDT 2014
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.
--paulr
> -----Original Message-----
> From: cfe-commits-bounces at cs.uiuc.edu [mailto:cfe-commits-
> bounces at cs.uiuc.edu] On Behalf Of David Blaikie
> Sent: Friday, May 09, 2014 7:45 PM
> To: cfe-commits at cs.uiuc.edu
> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list