r208468 - Add FIXME describing the limitation of using column info to disambiguate inlining.
David Blaikie
dblaikie at gmail.com
Fri May 9 19:44:57 PDT 2014
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]] =
More information about the cfe-commits
mailing list