r208468 - Add FIXME describing the limitation of using column info to disambiguate inlining.
David Blaikie
dblaikie at gmail.com
Mon May 12 11:55:30 PDT 2014
On Mon, May 12, 2014 at 11:48 AM, Robinson, Paul
<Paul_Robinson at playstation.sony.com> 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: 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