[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled
This revision was automatically updated to reflect the committed changes.
Closed by commit rL312965: [codeview] omit debug locations for nested exprs
unless column info enabled (authored by inglorion).
Changed prior to commit:
https://reviews.llvm.org/D37529?vs=114715=114716#toc
Repository:
rL LLVM
https://reviews.llvm.org/D37529
Files:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.h
cfe/trunk/test/CodeGenCXX/debug-info-nested-exprs.cpp
Index: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp
@@ -3553,6 +3553,10 @@
return ConstantAddress(GV, Alignment);
}
+bool CodeGenModule::getExpressionLocationsEnabled() const {
+ return !CodeGenOpts.EmitCodeView || CodeGenOpts.DebugColumnInfo;
+}
+
QualType CodeGenModule::getObjCFastEnumerationStateType() {
if (ObjCFastEnumerationStateType.isNull()) {
RecordDecl *D = Context.buildImplicitRecord("__objcFastEnumerationState");
Index: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
@@ -97,6 +97,10 @@
}
OriginalLocation = CGF->Builder.getCurrentDebugLocation();
+
+ if (OriginalLocation && !DI->CGM.getExpressionLocationsEnabled())
+return;
+
if (TemporaryLocation.isValid()) {
DI->EmitLocation(CGF->Builder, TemporaryLocation);
return;
Index: cfe/trunk/lib/CodeGen/CodeGenModule.h
===
--- cfe/trunk/lib/CodeGen/CodeGenModule.h
+++ cfe/trunk/lib/CodeGen/CodeGenModule.h
@@ -513,6 +513,9 @@
/// Finalize LLVM code generation.
void Release();
+ /// Return true if we should emit location information for expressions.
+ bool getExpressionLocationsEnabled() const;
+
/// Return a reference to the configured Objective-C runtime.
CGObjCRuntime () {
if (!ObjCRuntime) createObjCRuntime();
Index: cfe/trunk/test/CodeGenCXX/debug-info-nested-exprs.cpp
===
--- cfe/trunk/test/CodeGenCXX/debug-info-nested-exprs.cpp
+++ cfe/trunk/test/CodeGenCXX/debug-info-nested-exprs.cpp
@@ -0,0 +1,202 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-std=c++11 -gcodeview -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=NONEST %s
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-std=c++11 -gcodeview -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-std=c++11 -emit-llvm -o - %s | FileCheck -check-prefix=NESTED %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-std=c++11 -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+
+class Foo {
+public:
+ static Foo create();
+ void func();
+ int *begin();
+ int *end();
+};
+
+int bar(int x, int y);
+int baz(int x, int y);
+int qux(int x, int y);
+int onearg(int x);
+int noargs();
+int noargs1();
+Foo range(int x);
+
+int foo(int x, int y, int z) {
+ int a = bar(x, y) +
+ baz(x, z) +
+ qux(y, z);
+ // NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+ // NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]
+ // NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[LOC]]
+ // NONEST: store i32 {{.*}}, i32* %a,{{.*}} !dbg ![[LOC]]
+ // NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+ // NESTED: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+ // NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+ // NESTED: store i32 {{.*}}, i32* %a,{{.*}} !dbg ![[BAR]]
+ // COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+ // COLUMNS: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+ // COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+ // COLUMNS: store i32 {{.*}}, i32* %a,{{.*}} !dbg ![[DECLA:[0-9]+]]
+
+ int i = 1, b = 0, c = 0;
+ // NONEST: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+ // NONEST: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+ // NONEST: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+ // NESTED: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+ // NESTED: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+ // NESTED: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+ // COLUMNS: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+ // COLUMNS: store i32 0, i32* %b,{{.*}} !dbg ![[BLOC:[0-9]+]]
+ // COLUMNS: store i32 0, i32* %c,{{.*}} !dbg ![[CLOC:[0-9]+]]
+
+ while (i > 0) {
+b = bar(a, b);
+--i;
+ }
+ // NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+ // NONEST: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+ //
[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled
inglorion updated this revision to Diff 114715.
inglorion added a comment.
renamed get{Nested,}ExpressionLocationsEnabled and moved it into
CodeGetModule.cpp
https://reviews.llvm.org/D37529
Files:
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/CodeGen/CodeGenModule.h
clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
Index: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
@@ -0,0 +1,202 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-std=c++11 -gcodeview -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=NONEST %s
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-std=c++11 -gcodeview -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-std=c++11 -emit-llvm -o - %s | FileCheck -check-prefix=NESTED %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-std=c++11 -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+
+class Foo {
+public:
+ static Foo create();
+ void func();
+ int *begin();
+ int *end();
+};
+
+int bar(int x, int y);
+int baz(int x, int y);
+int qux(int x, int y);
+int onearg(int x);
+int noargs();
+int noargs1();
+Foo range(int x);
+
+int foo(int x, int y, int z) {
+ int a = bar(x, y) +
+ baz(x, z) +
+ qux(y, z);
+ // NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+ // NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]
+ // NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[LOC]]
+ // NONEST: store i32 {{.*}}, i32* %a,{{.*}} !dbg ![[LOC]]
+ // NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+ // NESTED: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+ // NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+ // NESTED: store i32 {{.*}}, i32* %a,{{.*}} !dbg ![[BAR]]
+ // COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+ // COLUMNS: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+ // COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+ // COLUMNS: store i32 {{.*}}, i32* %a,{{.*}} !dbg ![[DECLA:[0-9]+]]
+
+ int i = 1, b = 0, c = 0;
+ // NONEST: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+ // NONEST: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+ // NONEST: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+ // NESTED: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+ // NESTED: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+ // NESTED: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+ // COLUMNS: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+ // COLUMNS: store i32 0, i32* %b,{{.*}} !dbg ![[BLOC:[0-9]+]]
+ // COLUMNS: store i32 0, i32* %c,{{.*}} !dbg ![[CLOC:[0-9]+]]
+
+ while (i > 0) {
+b = bar(a, b);
+--i;
+ }
+ // NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+ // NONEST: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+ // NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+ // NESTED: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+ // COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+ // COLUMNS: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+
+ for (i = 0; i < 1; i++) {
+b = bar(a, b);
+c = qux(a, c);
+ }
+ // NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+ // NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+ // NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+ // NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+ // COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+ // COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+
+ if (a < b) {
+int t = a;
+a = b;
+b = t;
+ }
+ // NONEST: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+ // NONEST: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+ // NONEST: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+ // NESTED: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+ // NESTED: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+ // NESTED: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+ // COLUMNS: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+ // COLUMNS: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+ // COLUMNS: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+
+ int d = onearg(
+ noargs());
+ // NONEST: call i32 @{{.*}}noargs{{.*}}, !dbg ![[DECLD:[0-9]+]]
+ // NONEST: call i32 @{{.*}}onearg{{.*}}, !dbg ![[DECLD]]
+ // NONEST: store i32 %{{[^,]+}}, i32* %d,{{.*}} !dbg ![[DECLD]]
+ // NESTED: call i32 @{{.*}}noargs{{.*}}, !dbg ![[DNOARGS:[0-9]+]]
+ // NESTED: call i32 @{{.*}}onearg{{.*}}, !dbg ![[DECLD:[0-9]+]]
+ //
[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled
rnk added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.h:32 #include "clang/Basic/XRayLists.h" +#include "clang/Frontend/CodeGenOptions.h" #include "llvm/ADT/DenseMap.h" majnemer wrote: > Any reason to do this? I'd just keep getNestedExpressionLocationsEnabled in > the .cpp file. +1, I think we can continue to expect that ApplyDebugLocation will be used for expression emission and CodeGenFunction::EmitStopPoint (or whatever we rename it to) will be called for statements in CGStmt. https://reviews.llvm.org/D37529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled
majnemer added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.h:32 #include "clang/Basic/XRayLists.h" +#include "clang/Frontend/CodeGenOptions.h" #include "llvm/ADT/DenseMap.h" Any reason to do this? I'd just keep getNestedExpressionLocationsEnabled in the .cpp file. https://reviews.llvm.org/D37529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
Comment at: clang/lib/CodeGen/CodeGenModule.h:517
+ /// Return true if we should emit location information for nested
expressions.
+ bool getNestedExpressionLocationsEnabled() const {
+return !CodeGenOpts.EmitCodeView || CodeGenOpts.DebugColumnInfo;
I'd drop the "nested" part of this name. Statements are also nested, in a
sense. A for loop is a statement, and it has child statements. This is really
about expression locations vs. statement expressions.
https://reviews.llvm.org/D37529
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled
inglorion updated this revision to Diff 114465.
inglorion added a comment.
Of course, ApplyDebugLocation is also a perfectly legitimate way to add a debug
location to nodes that are not nested inside nodes that already have a
location. I updated the diff so that we do end up applying the location in such
cases.
https://reviews.llvm.org/D37529
Files:
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CodeGenModule.h
clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
Index: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
@@ -0,0 +1,202 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-std=c++11 -gcodeview -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=NONEST %s
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-std=c++11 -gcodeview -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-std=c++11 -emit-llvm -o - %s | FileCheck -check-prefix=NESTED %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-std=c++11 -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+
+class Foo {
+public:
+ static Foo create();
+ void func();
+ int *begin();
+ int *end();
+};
+
+int bar(int x, int y);
+int baz(int x, int y);
+int qux(int x, int y);
+int onearg(int x);
+int noargs();
+int noargs1();
+Foo range(int x);
+
+int foo(int x, int y, int z) {
+ int a = bar(x, y) +
+ baz(x, z) +
+ qux(y, z);
+ // NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+ // NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]
+ // NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[LOC]]
+ // NONEST: store i32 {{.*}}, i32* %a,{{.*}} !dbg ![[LOC]]
+ // NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+ // NESTED: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+ // NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+ // NESTED: store i32 {{.*}}, i32* %a,{{.*}} !dbg ![[BAR]]
+ // COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+ // COLUMNS: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+ // COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+ // COLUMNS: store i32 {{.*}}, i32* %a,{{.*}} !dbg ![[DECLA:[0-9]+]]
+
+ int i = 1, b = 0, c = 0;
+ // NONEST: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+ // NONEST: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+ // NONEST: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+ // NESTED: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+ // NESTED: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+ // NESTED: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+ // COLUMNS: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+ // COLUMNS: store i32 0, i32* %b,{{.*}} !dbg ![[BLOC:[0-9]+]]
+ // COLUMNS: store i32 0, i32* %c,{{.*}} !dbg ![[CLOC:[0-9]+]]
+
+ while (i > 0) {
+b = bar(a, b);
+--i;
+ }
+ // NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+ // NONEST: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+ // NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+ // NESTED: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+ // COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+ // COLUMNS: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+
+ for (i = 0; i < 1; i++) {
+b = bar(a, b);
+c = qux(a, c);
+ }
+ // NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+ // NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+ // NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+ // NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+ // COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+ // COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+
+ if (a < b) {
+int t = a;
+a = b;
+b = t;
+ }
+ // NONEST: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+ // NONEST: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+ // NONEST: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+ // NESTED: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+ // NESTED: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+ // NESTED: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+ // COLUMNS: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+ // COLUMNS: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+ // COLUMNS: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+
+ int d = onearg(
+ noargs());
+ // NONEST: call i32 @{{.*}}noargs{{.*}}, !dbg ![[DECLD:[0-9]+]]
+ // NONEST: call i32 @{{.*}}onearg{{.*}}, !dbg ![[DECLD]]
+ // NONEST: store i32 %{{[^,]+}}, i32* %d,{{.*}} !dbg ![[DECLD]]
+ // NESTED: call i32
Re: [PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled
Well, if they worked I wasn't going to say we needed to add tests for them, i just wanted to make sure they work before we move onto something else. In any case, lgtm On Fri, Sep 8, 2017 at 4:43 PM Bob Haarman via Phabricator < revi...@reviews.llvm.org> wrote: > inglorion updated this revision to Diff 114463. > inglorion added a comment. > > added examples suggested by @zturner, verified step over and step into > specific behavior matches MSVC, and added tests for them > > > https://reviews.llvm.org/D37529 > > Files: > clang/lib/CodeGen/CGDebugInfo.cpp > clang/lib/CodeGen/CodeGenModule.h > clang/test/CodeGenCXX/debug-info-nested-exprs.cpp > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled
inglorion updated this revision to Diff 114463.
inglorion added a comment.
added examples suggested by @zturner, verified step over and step into specific
behavior matches MSVC, and added tests for them
https://reviews.llvm.org/D37529
Files:
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CodeGenModule.h
clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
Index: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
@@ -0,0 +1,202 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-std=c++11 -gcodeview -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=NONEST %s
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-std=c++11 -gcodeview -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-std=c++11 -emit-llvm -o - %s | FileCheck -check-prefix=NESTED %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-std=c++11 -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+
+class Foo {
+public:
+ static Foo create();
+ void func();
+ int *begin();
+ int *end();
+};
+
+int bar(int x, int y);
+int baz(int x, int y);
+int qux(int x, int y);
+int onearg(int x);
+int noargs();
+int noargs1();
+Foo range(int x);
+
+int foo(int x, int y, int z) {
+ int a = bar(x, y) +
+ baz(x, z) +
+ qux(y, z);
+ // NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+ // NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]
+ // NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[LOC]]
+ // NONEST: store i32 {{.*}}, i32* %a,{{.*}} !dbg ![[LOC]]
+ // NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+ // NESTED: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+ // NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+ // NESTED: store i32 {{.*}}, i32* %a,{{.*}} !dbg ![[BAR]]
+ // COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+ // COLUMNS: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+ // COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+ // COLUMNS: store i32 {{.*}}, i32* %a,{{.*}} !dbg ![[DECLA:[0-9]+]]
+
+ int i = 1, b = 0, c = 0;
+ // NONEST: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+ // NONEST: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+ // NONEST: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+ // NESTED: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+ // NESTED: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+ // NESTED: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+ // COLUMNS: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+ // COLUMNS: store i32 0, i32* %b,{{.*}} !dbg ![[BLOC:[0-9]+]]
+ // COLUMNS: store i32 0, i32* %c,{{.*}} !dbg ![[CLOC:[0-9]+]]
+
+ while (i > 0) {
+b = bar(a, b);
+--i;
+ }
+ // NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+ // NONEST: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+ // NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+ // NESTED: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+ // COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+ // COLUMNS: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+
+ for (i = 0; i < 1; i++) {
+b = bar(a, b);
+c = qux(a, c);
+ }
+ // NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+ // NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+ // NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+ // NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+ // COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+ // COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+
+ if (a < b) {
+int t = a;
+a = b;
+b = t;
+ }
+ // NONEST: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+ // NONEST: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+ // NONEST: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+ // NESTED: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+ // NESTED: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+ // NESTED: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+ // COLUMNS: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+ // COLUMNS: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+ // COLUMNS: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+
+ int d = onearg(
+ noargs());
+ // NONEST: call i32 @{{.*}}noargs{{.*}}, !dbg ![[DECLD:[0-9]+]]
+ // NONEST: call i32 @{{.*}}onearg{{.*}}, !dbg ![[DECLD]]
+ // NONEST: store i32 %{{[^,]+}}, i32* %d,{{.*}} !dbg ![[DECLD]]
+ // NESTED: call i32 @{{.*}}noargs{{.*}}, !dbg ![[DNOARGS:[0-9]+]]
+ // NESTED: call i32 @{{.*}}onearg{{.*}}, !dbg ![[DECLD:[0-9]+]]
[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled
zturner added a comment. @inglorion : Since we're already in this code anyway, should we at least do due diligence and check the basics, even if only manually? As in, at least we should check in MSVC if any of those other types of examples that I came up with present a problem. Otherwise we're going to be back here fixing this again in a few weeks. And it doesn't seem like it would be that hard to whip up some sample code with 10 or 12 cases and just do a sanity check that they all work. If they don't, we can then decide whether it should be fixed in this patch, or a bug can be filed to track it. https://reviews.llvm.org/D37529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled
inglorion updated this revision to Diff 114299.
inglorion added a comment.
removed compound statement; that was only in there to double check that the
debugger stops on the first child statement, but that's true with or without
this change
https://reviews.llvm.org/D37529
Files:
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CodeGenModule.h
clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
Index: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
@@ -0,0 +1,110 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-gcodeview -emit-llvm -o - %s | FileCheck -check-prefix=NONEST %s
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-gcodeview -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-emit-llvm -o - %s | FileCheck -check-prefix=NESTED %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+// NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]
+// NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[LOC]]
+// NONEST: store i32 {{.*}}, !dbg ![[LOC]]
+// NONEST: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+// NONEST: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+// NONEST: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+// NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+// NONEST: mul nsw i32 {{.*}}, !dbg ![[RETLOC:[0-9]+]]
+// NONEST: sub nsw i32 {{.*}}, !dbg ![[RETLOC]]
+// NONEST: ret i32 {{.*}}, !dbg ![[RETLOC]]
+// NONEST: ![[WHILE1]] = !DILocation(
+// NONEST: ![[WHILE2]] = !DILocation(
+// NONEST: ![[FOR1]] = !DILocation(
+// NONEST: ![[FOR2]] = !DILocation(
+// NONEST: ![[IF1]] = !DILocation(
+// NONEST: ![[IF2]] = !DILocation(
+// NONEST: ![[IF3]] = !DILocation(
+
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+// NESTED: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+// NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+// NESTED: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+// NESTED: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+// NESTED: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+// NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+// NESTED: mul nsw i32 {{.*}}, !dbg ![[RETMUL:[0-9]+]]
+// NESTED: sub nsw i32 {{.*}}, !dbg ![[RETSUB:[0-9]+]]
+// NESTED: ret i32 {{.*}}, !dbg !
+// NESTED: ![[BAR]] = !DILocation(
+// NESTED: ![[BAZ]] = !DILocation(
+// NESTED: ![[QUX]] = !DILocation(
+// NESTED: ![[RETSUB]] = !DILocation(
+// NESTED: ![[RETMUL]] = !DILocation(
+
+// COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+// COLUMNS: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+// COLUMNS: store i32 0, i32* %b,{{.*}} !dbg ![[BLOC:[0-9]+]]
+// COLUMNS: store i32 0, i32* %c,{{.*}} !dbg ![[CLOC:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+// COLUMNS: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+// COLUMNS: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+// COLUMNS: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+// COLUMNS: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+// COLUMNS: mul nsw i32 {{.*}}, !dbg ![[RETMUL:[0-9]+]]
+// COLUMNS: sub nsw i32 {{.*}}, !dbg ![[RETSUB:[0-9]+]]
+// COLUMNS: ret i32 {{.*}}, !dbg !
+// COLUMNS: ![[BAR]] = !DILocation(
+// COLUMNS: ![[BAZ]] = !DILocation(
+// COLUMNS: ![[QUX]] = !DILocation(
+// COLUMNS: ![[ILOC]] = !DILocation(
+// COLUMNS: ![[BLOC]] = !DILocation(
+// COLUMNS: ![[CLOC]] = !DILocation(
+// COLUNMS: ![[RETSUB]] = !DILocation(
+// COLUMNS:
[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled
inglorion added a comment. @zturner: I am still thinking about your comment about other cases to test. My concern is that there are very many possible combinations. I'm actually not too concerned about not exactly matching cl's behavior in every single case. The difference in behavior here is us emitting a debug location for an expression that doesn't get its own debug location from cl. In general, I think having more fine-grained information is good, so I don't think differing in this way is a problem. That is, unless we end up breaking functionality in the debugger (Visual Studio). The behavior I know of we can end up breaking this way is step into specific, which appears to require multiple calls that are associated with a single debug location. I think, at a minimum, the test case should cover a scenario where we would normally like to emit some debug locations, but need to elide them if we want to be compatible with Visual Studio. I also think it makes sense to include some cases where we want the behavior to be the same whether or not we're targeting MS compatibility. That way, we can verify that we aren't throwing away too much information. Beyond that, I feel there are diminishing returns. To avoid going too far down that path, I would like to start with a relatively small test case (as I've done), fix the bug that prompted me to write this code, and then add additional tests if we find out there are other cases where people care strongly about the granularity of the debug locations we emit. Does that sound reasonable? https://reviews.llvm.org/D37529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled
inglorion updated this revision to Diff 114292.
inglorion added a comment.
Following conversation with @rnk, I managed to whittle this down to a very
small change that seems to do what we need. Step into specific works and single
stepping through the program behaves similarly whether compiled with clang-cl
or cl.
https://reviews.llvm.org/D37529
Files:
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CodeGenModule.h
clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
Index: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
@@ -0,0 +1,115 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-gcodeview -emit-llvm -o - %s | FileCheck -check-prefix=NONEST %s
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-gcodeview -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-emit-llvm -o - %s | FileCheck -check-prefix=NESTED %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+// NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]
+// NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[LOC]]
+// NONEST: store i32 {{.*}}, !dbg ![[LOC]]
+// NONEST: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+// NONEST: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+// NONEST: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+// NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+// NONEST: mul nsw i32 {{.*}}, !dbg ![[RETLOC:[0-9]+]]
+// NONEST: sub nsw i32 {{.*}}, !dbg ![[RETLOC]]
+// NONEST: ret i32 {{.*}}, !dbg ![[RETLOC]]
+// NONEST: ![[WHILE1]] = !DILocation(
+// NONEST: ![[WHILE2]] = !DILocation(
+// NONEST: ![[FOR1]] = !DILocation(
+// NONEST: ![[FOR2]] = !DILocation(
+// NONEST: ![[IF1]] = !DILocation(
+// NONEST: ![[IF2]] = !DILocation(
+// NONEST: ![[IF3]] = !DILocation(
+
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+// NESTED: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+// NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+// NESTED: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+// NESTED: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+// NESTED: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+// NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+// NESTED: mul nsw i32 {{.*}}, !dbg ![[RETMUL:[0-9]+]]
+// NESTED: sub nsw i32 {{.*}}, !dbg ![[RETSUB:[0-9]+]]
+// NESTED: ret i32 {{.*}}, !dbg !
+// NESTED: ![[BAR]] = !DILocation(
+// NESTED: ![[BAZ]] = !DILocation(
+// NESTED: ![[QUX]] = !DILocation(
+// NESTED: ![[RETSUB]] = !DILocation(
+// NESTED: ![[RETMUL]] = !DILocation(
+
+// COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+// COLUMNS: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+// COLUMNS: store i32 0, i32* %b,{{.*}} !dbg ![[BLOC:[0-9]+]]
+// COLUMNS: store i32 0, i32* %c,{{.*}} !dbg ![[CLOC:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+// COLUMNS: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+// COLUMNS: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+// COLUMNS: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+// COLUMNS: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+// COLUMNS: mul nsw i32 {{.*}}, !dbg ![[RETMUL:[0-9]+]]
+// COLUMNS: sub nsw i32 {{.*}}, !dbg ![[RETSUB:[0-9]+]]
+// COLUMNS: ret i32 {{.*}}, !dbg !
+// COLUMNS: ![[BAR]] = !DILocation(
+// COLUMNS: ![[BAZ]] = !DILocation(
+// COLUMNS: ![[QUX]] = !DILocation(
+// COLUMNS: ![[ILOC]] = !DILocation(
+// COLUMNS: ![[BLOC]] = !DILocation(
+// COLUMNS: ![[CLOC]] =
[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled
inglorion planned changes to this revision. inglorion added a comment. rnk and I talked about a different approach. The idea is to explicitly emit locations in some cases (e.g. inside compound statements, the braces of for loops, ...), and otherwise emit locations only when emitting column info or emitting non-codeview debug info. That may lead to more elegant code. I'll give it a try later. https://reviews.llvm.org/D37529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled
inglorion updated this revision to Diff 114097.
inglorion marked 5 inline comments as done.
inglorion added a comment.
I limited the change to only calls, returns, and declarations. I also
updated the test case to include a multi-variable declaration, a while
loop, a for loop, and an if statement (after verifying the behavior in
the debugger, compared to MSVC). I discovered that there is a
difference between the generated info for DWARF with or without
-dwarf-column-info, so I included that in the test, too. I also made a
couple of minor changes that were suggested.
https://reviews.llvm.org/D37529
Files:
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CGDebugInfo.h
clang/lib/CodeGen/CGException.cpp
clang/lib/CodeGen/CGStmt.cpp
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/lib/CodeGen/CodeGenModule.h
clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
Index: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
@@ -0,0 +1,110 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-gcodeview -emit-llvm -o - %s | FileCheck -check-prefix=NONEST %s
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-gcodeview -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-emit-llvm -o - %s | FileCheck -check-prefix=NESTED %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+// NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]
+// NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[LOC]]
+// NONEST: store i32 {{.*}}, !dbg ![[LOC]]
+// NONEST: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+// NONEST: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+// NONEST: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+// NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+// NONEST: mul nsw i32 {{.*}}, !dbg ![[RETLOC:[0-9]+]]
+// NONEST: sub nsw i32 {{.*}}, !dbg ![[RETLOC]]
+// NONEST: ret i32 {{.*}}, !dbg ![[RETLOC]]
+// NONEST: ![[WHILE1]] = !DILocation(
+// NONEST: ![[WHILE2]] = !DILocation(
+// NONEST: ![[FOR1]] = !DILocation(
+// NONEST: ![[FOR2]] = !DILocation(
+// NONEST: ![[IF1]] = !DILocation(
+// NONEST: ![[IF2]] = !DILocation(
+// NONEST: ![[IF3]] = !DILocation(
+
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+// NESTED: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+// NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+// NESTED: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+// NESTED: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+// NESTED: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+// NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+// NESTED: mul nsw i32 {{.*}}, !dbg ![[RETMUL:[0-9]+]]
+// NESTED: sub nsw i32 {{.*}}, !dbg ![[RETSUB:[0-9]+]]
+// NESTED: ret i32 {{.*}}, !dbg !
+// NESTED: ![[BAR]] = !DILocation(
+// NESTED: ![[BAZ]] = !DILocation(
+// NESTED: ![[QUX]] = !DILocation(
+// NESTED: ![[RETSUB]] = !DILocation(
+// NESTED: ![[RETMUL]] = !DILocation(
+
+// COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+// COLUMNS: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+// COLUMNS: store i32 0, i32* %b,{{.*}} !dbg ![[BLOC:[0-9]+]]
+// COLUMNS: store i32 0, i32* %c,{{.*}} !dbg ![[CLOC:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+// COLUMNS: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+// COLUMNS: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+// COLUMNS: store i32
[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled
zturner added inline comments. Comment at: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp:44 + int a = bar(x, y) + + baz(x, z) + + qux(y, z); inglorion wrote: > zturner wrote: > > inglorion wrote: > > > zturner wrote: > > > > Can you make a function called `int foo()` and make this `int a = > > > > bar(foo(), y) + ...` > > > Yes. Why? To test an additional level of nesting? > > Yes. for that matter, even better would be if this call to `foo()` spans > > multiple lines. Right here you've got a single statement which spans > > multiple lines, but no individual sub-expression spans multiple lines. And > > FWICT there is no nesting at all, since + is just a builtin operator. > The nesting here is the calls to bar, baz, and qux inside the declaration of > a. The old behavior would emit separate locations for each of the calls, the > new behavior annotates each of the calls with the same location as the > declaration. This causes the debugger to stop only once for the entire > statement when using step over, and makes step into specific work. Sure, but does that execute the same codepath as: ``` int a = bar( baz()); a = bar(baz()); for (const auto : foo(bar())) for (int x = foo(); x < bar(); baz(x)) if (foo() && bar()) if (createObject().func()) ``` etc? And what if one or more of these functions get inlined? For example, what if you have: ``` int a = foo(bar(baz()), bar(buzz())); ``` https://reviews.llvm.org/D37529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled
zturner added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.h:65 llvm::MDNode *CurInlinedAt = nullptr; + bool LocationEnabled = true; llvm::DIType *VTablePtrType = nullptr; Can you move this line up to put it next to another bool? Not a huge deal, but might as well pack the class members. Comment at: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp:44 + int a = bar(x, y) + + baz(x, z) + + qux(y, z); Can you make a function called `int foo()` and make this `int a = bar(foo(), y) + ...` https://reviews.llvm.org/D37529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled
inglorion added inline comments. Comment at: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp:44 + int a = bar(x, y) + + baz(x, z) + + qux(y, z); zturner wrote: > inglorion wrote: > > zturner wrote: > > > Can you make a function called `int foo()` and make this `int a = > > > bar(foo(), y) + ...` > > Yes. Why? To test an additional level of nesting? > Yes. for that matter, even better would be if this call to `foo()` spans > multiple lines. Right here you've got a single statement which spans > multiple lines, but no individual sub-expression spans multiple lines. And > FWICT there is no nesting at all, since + is just a builtin operator. The nesting here is the calls to bar, baz, and qux inside the declaration of a. The old behavior would emit separate locations for each of the calls, the new behavior annotates each of the calls with the same location as the declaration. This causes the debugger to stop only once for the entire statement when using step over, and makes step into specific work. https://reviews.llvm.org/D37529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled
inglorion created this revision.
Herald added a subscriber: aprantl.
Microsoft Visual Studio expects debug locations to correspond to
statements. We used to emit locations for expressions nested inside statements.
This would confuse the debugger, causing it to stop multiple times on the
same line and breaking the "step into specific" feature. This change inhibits
the emission of debug locations for nested expressions when emitting CodeView
debug information, unless column information is enabled.
Fixes PR34312.
https://reviews.llvm.org/D37529
Files:
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CGDebugInfo.h
clang/lib/CodeGen/CGStmt.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/lib/CodeGen/CodeGenModule.h
clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
Index: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-gcodeview -emit-llvm -o - %s | FileCheck -check-prefix=NONEST %s
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-gcodeview -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=NESTED %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-emit-llvm -o - %s | FileCheck -check-prefix=NESTED %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=NESTED %s
+
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+// NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]
+// NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[LOC]]
+// NONEST: store i32 {{.*}}, !dbg ![[LOC]]
+// NONEST: mul nsw i32 {{.*}}, !dbg ![[RETLOC:[0-9]+]]
+// NONEST: sub nsw i32 {{.*}}, !dbg ![[RETLOC]]
+// NONEST: ret i32 {{.*}}, !dbg ![[RETLOC]]
+
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+// NESTED: call i32 @{{.*}}baz{{.*}}, !dbg !
+// NESTED-NOT: [[LOC]]
+// NESTED-SAME: {{[0-9]+}}
+// NESTED: call i32 @{{.*}}qux{{.*}}, !dbg !
+// NESTED-NOT: [[LOC]]
+// NESTED-SAME: {{[0-9]+}}
+// NESTED: store i32 {{.*}}, !dbg !
+// NESTED-NOT: [[LOC]]
+// NESTED-SAME: {{[0-9]+}}
+// NESTED: mul nsw i32 {{.*}}, !dbg ![[RETLOC:[0-9]+]]
+// NESTED: sub nsw i32 {{.*}}, !dbg !
+// NESTED-NOT: [[RETLOC]]
+// NESTED-SAME: {{[0-9]+}}
+// NESTED: ret i32 {{.*}}, !dbg !
+// NESTED-NOT: [[RETLOC]]
+// NESTED-SAME: {{[0-9]+}}
+
+int bar(int x, int y);
+int baz(int x, int y);
+int qux(int x, int y);
+
+int foo(int x, int y, int z) {
+ int a = bar(x, y) +
+ baz(x, z) +
+ qux(y, z);
+ return a -
+ (y * z);
+}
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -29,6 +29,7 @@
#include "clang/Basic/Module.h"
#include "clang/Basic/SanitizerBlacklist.h"
#include "clang/Basic/XRayLists.h"
+#include "clang/Frontend/CodeGenOptions.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/SetVector.h"
#include "llvm/ADT/SmallPtrSet.h"
@@ -70,7 +71,6 @@
class ValueDecl;
class VarDecl;
class LangOptions;
-class CodeGenOptions;
class HeaderSearchOptions;
class PreprocessorOptions;
class DiagnosticsEngine;
@@ -513,6 +513,11 @@
/// Finalize LLVM code generation.
void Release();
+ /// Return true if we should emit location information for nested expressions.
+ bool getNestedExpressionLocationsEnabled() const {
+return !CodeGenOpts.EmitCodeView || CodeGenOpts.DebugColumnInfo;
+ }
+
/// Return a reference to the configured Objective-C runtime.
CGObjCRuntime () {
if (!ObjCRuntime) createObjCRuntime();
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -87,6 +87,7 @@
class BlockByrefInfo;
class BlockFlags;
class BlockFieldFlags;
+class InhibitDebugLocation;
class RegionCodeGenTy;
class TargetCodeGenInfo;
struct OMPTaskDataTy;
@@ -2525,6 +2526,14 @@
// Statement Emission
//======//
+ /// EmitStmtStopPoint - Emit a statement stoppoint if we are emitting debug
+ /// info.
+ /// If getNestedExpressionLocationsEnabled() returns false, this also inhibits
+ /// the emission of debug location information until the returned
+ /// InhibitDebugLocation
+ /// object is destroyed.
+ InhibitDebugLocation EmitStmtStopPoint(const Stmt *S);
+
/// EmitStopPoint - Emit a debug stoppoint if we are emitting debug info.
void EmitStopPoint(const Stmt *S);
Index: clang/lib/CodeGen/CGStmt.cpp
[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled
inglorion added inline comments. Comment at: clang/lib/CodeGen/CGStmt.cpp:45 + } + return IDL; +} inglorion wrote: > rnk wrote: > > Does MSVC accept this? I think it will emit the copy ctor call in an -O0 > > build. > I wrote this thinking that the right thing would happen under copy elision > (there is only one object, move constructor isn't called, and the destructor > only runs once) and without copy elision (there are two objects, move > constructor is called, destructor is run for both objects but is a no-op for > the moved-from object). If that's not the case, how would you rewrite this to > do the right thing? FWIW, the test passes with MSVC in a Debug build, too. https://reviews.llvm.org/D37529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled
zturner added inline comments. Comment at: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp:44 + int a = bar(x, y) + + baz(x, z) + + qux(y, z); inglorion wrote: > zturner wrote: > > Can you make a function called `int foo()` and make this `int a = > > bar(foo(), y) + ...` > Yes. Why? To test an additional level of nesting? Yes. for that matter, even better would be if this call to `foo()` spans multiple lines. Right here you've got a single statement which spans multiple lines, but no individual sub-expression spans multiple lines. And FWICT there is no nesting at all, since + is just a builtin operator. https://reviews.llvm.org/D37529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled
rnk added inline comments.
Comment at: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp:12
+
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+// NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]
inglorion wrote:
> rnk wrote:
> > This is pretty painful to test. :(
> >
> > If we let ourselves do an asm test, .cv_loc is printed with some nice
> > assembly comments that make this testing easy.
> Would you like me to write an asm test in addition to / instead of this one?
Oops, I thought I wrote this comment and deleted it before hitting send, but
apparently not. No, let's stick with the IR test.
https://reviews.llvm.org/D37529
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled
rnk added a subscriber: echristo.
rnk added inline comments.
Comment at: clang/lib/CodeGen/CGStmt.cpp:38
+InhibitDebugLocation CodeGenFunction::EmitStmtStopPoint(const Stmt *S) {
+ InhibitDebugLocation IDL;
"Stop point" is a hold-over from the llvm.dbg.stoppoint, which doesn't exist
anymore:
http://releases.llvm.org/2.6/docs/SourceLevelDebugging.html#format_common_stoppoint
@echristo renamed CGDebugInfo::EmitStopPoint to EmitLocation long ago, and we
should do the same for CodeGenFunction::EmitStopPoint. I'd like to have
something like `EmitStmtLocation` and `EmitExprLocation`.
Comment at: clang/lib/CodeGen/CGStmt.cpp:45
+ }
+ return IDL;
+}
Does MSVC accept this? I think it will emit the copy ctor call in an -O0 build.
Comment at: clang/lib/CodeGen/CGStmt.cpp:145
case Stmt::IfStmtClass: EmitIfStmt(cast(*S));
break;
case Stmt::WhileStmtClass:EmitWhileStmt(cast(*S));
break;
Doesn't this end up recursing? Won't InhibitDebugLocation prevent us from
applying the inner statement locations?
Comment at: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp:12
+
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+// NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]
This is pretty painful to test. :(
If we let ourselves do an asm test, .cv_loc is printed with some nice assembly
comments that make this testing easy.
Comment at: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp:48
+ (y * z);
+}
I'd like to see some compound statement tests: for, while, if, regular braces,
etc.
https://reviews.llvm.org/D37529
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled
inglorion added inline comments.
Comment at: clang/lib/CodeGen/CGStmt.cpp:45
+ }
+ return IDL;
+}
rnk wrote:
> Does MSVC accept this? I think it will emit the copy ctor call in an -O0
> build.
I wrote this thinking that the right thing would happen under copy elision
(there is only one object, move constructor isn't called, and the destructor
only runs once) and without copy elision (there are two objects, move
constructor is called, destructor is run for both objects but is a no-op for
the moved-from object). If that's not the case, how would you rewrite this to
do the right thing?
Comment at: clang/lib/CodeGen/CGStmt.cpp:145
case Stmt::IfStmtClass: EmitIfStmt(cast(*S));
break;
case Stmt::WhileStmtClass:EmitWhileStmt(cast(*S));
break;
rnk wrote:
> Doesn't this end up recursing? Won't InhibitDebugLocation prevent us from
> applying the inner statement locations?
Yeah, this looks wrong. Let me get back to you with fixed code or an
explanation of why it actually does the right thing.
Comment at: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp:12
+
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+// NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]
rnk wrote:
> This is pretty painful to test. :(
>
> If we let ourselves do an asm test, .cv_loc is printed with some nice
> assembly comments that make this testing easy.
Would you like me to write an asm test in addition to / instead of this one?
Comment at: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp:44
+ int a = bar(x, y) +
+ baz(x, z) +
+ qux(y, z);
zturner wrote:
> Can you make a function called `int foo()` and make this `int a = bar(foo(),
> y) + ...`
Yes. Why? To test an additional level of nesting?
https://reviews.llvm.org/D37529
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled
inglorion updated this revision to Diff 114060.
inglorion added a comment.
removed accidentally left in include and reformatted mangled comment
https://reviews.llvm.org/D37529
Files:
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CGDebugInfo.h
clang/lib/CodeGen/CGStmt.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/lib/CodeGen/CodeGenModule.h
clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
Index: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-gcodeview -emit-llvm -o - %s | FileCheck -check-prefix=NONEST %s
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-gcodeview -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=NESTED %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-emit-llvm -o - %s | FileCheck -check-prefix=NESTED %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=NESTED %s
+
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+// NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]
+// NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[LOC]]
+// NONEST: store i32 {{.*}}, !dbg ![[LOC]]
+// NONEST: mul nsw i32 {{.*}}, !dbg ![[RETLOC:[0-9]+]]
+// NONEST: sub nsw i32 {{.*}}, !dbg ![[RETLOC]]
+// NONEST: ret i32 {{.*}}, !dbg ![[RETLOC]]
+
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+// NESTED: call i32 @{{.*}}baz{{.*}}, !dbg !
+// NESTED-NOT: [[LOC]]
+// NESTED-SAME: {{[0-9]+}}
+// NESTED: call i32 @{{.*}}qux{{.*}}, !dbg !
+// NESTED-NOT: [[LOC]]
+// NESTED-SAME: {{[0-9]+}}
+// NESTED: store i32 {{.*}}, !dbg !
+// NESTED-NOT: [[LOC]]
+// NESTED-SAME: {{[0-9]+}}
+// NESTED: mul nsw i32 {{.*}}, !dbg ![[RETLOC:[0-9]+]]
+// NESTED: sub nsw i32 {{.*}}, !dbg !
+// NESTED-NOT: [[RETLOC]]
+// NESTED-SAME: {{[0-9]+}}
+// NESTED: ret i32 {{.*}}, !dbg !
+// NESTED-NOT: [[RETLOC]]
+// NESTED-SAME: {{[0-9]+}}
+
+int bar(int x, int y);
+int baz(int x, int y);
+int qux(int x, int y);
+
+int foo(int x, int y, int z) {
+ int a = bar(x, y) +
+ baz(x, z) +
+ qux(y, z);
+ return a -
+ (y * z);
+}
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -29,6 +29,7 @@
#include "clang/Basic/Module.h"
#include "clang/Basic/SanitizerBlacklist.h"
#include "clang/Basic/XRayLists.h"
+#include "clang/Frontend/CodeGenOptions.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/SetVector.h"
#include "llvm/ADT/SmallPtrSet.h"
@@ -70,7 +71,6 @@
class ValueDecl;
class VarDecl;
class LangOptions;
-class CodeGenOptions;
class HeaderSearchOptions;
class PreprocessorOptions;
class DiagnosticsEngine;
@@ -513,6 +513,11 @@
/// Finalize LLVM code generation.
void Release();
+ /// Return true if we should emit location information for nested expressions.
+ bool getNestedExpressionLocationsEnabled() const {
+return !CodeGenOpts.EmitCodeView || CodeGenOpts.DebugColumnInfo;
+ }
+
/// Return a reference to the configured Objective-C runtime.
CGObjCRuntime () {
if (!ObjCRuntime) createObjCRuntime();
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -87,6 +87,7 @@
class BlockByrefInfo;
class BlockFlags;
class BlockFieldFlags;
+class InhibitDebugLocation;
class RegionCodeGenTy;
class TargetCodeGenInfo;
struct OMPTaskDataTy;
@@ -2525,6 +2526,12 @@
// Statement Emission
//======//
+ /// EmitStmtStopPoint - Emit a statement stoppoint if we are emitting debug
+ /// info. If getNestedExpressionLocationsEnabled() returns false, this also
+ /// inhibits the emission of debug location information until the returned
+ /// InhibitDebugLocation object is destroyed.
+ InhibitDebugLocation EmitStmtStopPoint(const Stmt *S);
+
/// EmitStopPoint - Emit a debug stoppoint if we are emitting debug info.
void EmitStopPoint(const Stmt *S);
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -35,8 +35,19 @@
// Statement Emission
//===--===//
+InhibitDebugLocation CodeGenFunction::EmitStmtStopPoint(const Stmt *S) {
+ InhibitDebugLocation IDL;
+ if (HaveInsertPoint()) {
+EmitStopPoint(S);
