<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Nov 20, 2019 at 1:08 AM Djordje Todorovic via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
Author: Djordje Todorovic<br>
Date: 2019-11-20T10:08:07+01:00<br>
New Revision: ce1f95a6e077693f93d8869245f911aff3eb7e4c<br>
<br>
URL: <a href="https://github.com/llvm/llvm-project/commit/ce1f95a6e077693f93d8869245f911aff3eb7e4c" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/ce1f95a6e077693f93d8869245f911aff3eb7e4c</a><br>
DIFF: <a href="https://github.com/llvm/llvm-project/commit/ce1f95a6e077693f93d8869245f911aff3eb7e4c.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/ce1f95a6e077693f93d8869245f911aff3eb7e4c.diff</a><br>
<br>
LOG: Reland "[clang] Remove the DIFlagArgumentNotModified debug info flag"<br>
<br>
It turns out that the ExprMutationAnalyzer can be very slow when AST<br>
gets huge in some cases. The idea is to move this analysis to the LLVM<br>
back-end level (more precisely, in the LiveDebugValues pass). The new<br>
approach will remove the performance regression, simplify the<br>
implementation and give us front-end independent implementation.<br></blockquote><div><br>What if the LLVM backend optimized out a dead store? (then we might concnlude that the argument is not modified, when it actually is modified?)<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Differential Revision: <a href="https://reviews.llvm.org/D68206" rel="noreferrer" target="_blank">https://reviews.llvm.org/D68206</a><br>
<br>
Added: <br>
<br>
<br>
Modified: <br>
clang/lib/CodeGen/CGDebugInfo.cpp<br>
clang/lib/CodeGen/CGDebugInfo.h<br>
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py<br>
<br>
Removed: <br>
clang/test/CodeGen/debug-info-param-modification.c<br>
<br>
<br>
################################################################################<br>
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp<br>
index 116517a9cb99..a9b3831aa0b5 100644<br>
--- a/clang/lib/CodeGen/CGDebugInfo.cpp<br>
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp<br>
@@ -18,7 +18,6 @@<br>
#include "CodeGenFunction.h"<br>
#include "CodeGenModule.h"<br>
#include "ConstantEmitter.h"<br>
-#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"<br>
#include "clang/AST/ASTContext.h"<br>
#include "clang/AST/DeclFriend.h"<br>
#include "clang/AST/DeclObjC.h"<br>
@@ -3686,15 +3685,6 @@ void CGDebugInfo::EmitFunctionStart(GlobalDecl GD, SourceLocation Loc,<br>
if (HasDecl && isa<FunctionDecl>(D))<br>
DeclCache[D->getCanonicalDecl()].reset(SP);<br>
<br>
- // We use the SPDefCache only in the case when the debug entry values option<br>
- // is set, in order to speed up parameters modification analysis.<br>
- //<br>
- // FIXME: Use AbstractCallee here to support ObjCMethodDecl.<br>
- if (CGM.getCodeGenOpts().EnableDebugEntryValues && HasDecl)<br>
- if (auto *FD = dyn_cast<FunctionDecl>(D))<br>
- if (FD->hasBody() && !FD->param_empty())<br>
- SPDefCache[FD].reset(SP);<br>
-<br>
// Push the function onto the lexical block stack.<br>
LexicalBlockStack.emplace_back(SP);<br>
<br>
@@ -4097,11 +4087,6 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const VarDecl *VD,<br>
llvm::DebugLoc::get(Line, Column, Scope, CurInlinedAt),<br>
Builder.GetInsertBlock());<br>
<br>
- if (CGM.getCodeGenOpts().EnableDebugEntryValues && ArgNo) {<br>
- if (auto *PD = dyn_cast<ParmVarDecl>(VD))<br>
- ParamCache[PD].reset(D);<br>
- }<br>
-<br>
return D;<br>
}<br>
<br>
@@ -4717,29 +4702,6 @@ void CGDebugInfo::setDwoId(uint64_t Signature) {<br>
TheCU->setDWOId(Signature);<br>
}<br>
<br>
-/// Analyzes each function parameter to determine whether it is constant<br>
-/// throughout the function body.<br>
-static void analyzeParametersModification(<br>
- ASTContext &Ctx,<br>
- llvm::DenseMap<const FunctionDecl *, llvm::TrackingMDRef> &SPDefCache,<br>
- llvm::DenseMap<const ParmVarDecl *, llvm::TrackingMDRef> &ParamCache) {<br>
- for (auto &SP : SPDefCache) {<br>
- auto *FD = SP.first;<br>
- assert(FD->hasBody() && "Functions must have body here");<br>
- const Stmt *FuncBody = (*FD).getBody();<br>
- for (auto Parm : FD->parameters()) {<br>
- ExprMutationAnalyzer FuncAnalyzer(*FuncBody, Ctx);<br>
- if (FuncAnalyzer.isMutated(Parm))<br>
- continue;<br>
-<br>
- auto I = ParamCache.find(Parm);<br>
- assert(I != ParamCache.end() && "Parameters should be already cached");<br>
- auto *DIParm = cast<llvm::DILocalVariable>(I->second);<br>
- DIParm->setIsNotModified();<br>
- }<br>
- }<br>
-}<br>
-<br>
void CGDebugInfo::finalize() {<br>
// Creating types might create further types - invalidating the current<br>
// element and the size(), so don't cache/reference them.<br>
@@ -4812,10 +4774,6 @@ void CGDebugInfo::finalize() {<br>
if (auto MD = TypeCache[RT])<br>
DBuilder.retainType(cast<llvm::DIType>(MD));<br>
<br>
- if (CGM.getCodeGenOpts().EnableDebugEntryValues)<br>
- // This will be used to emit debug entry values.<br>
- analyzeParametersModification(CGM.getContext(), SPDefCache, ParamCache);<br>
-<br>
DBuilder.finalize();<br>
}<br>
<br>
<br>
diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h<br>
index 9a097615b4b4..5341bfa7f350 100644<br>
--- a/clang/lib/CodeGen/CGDebugInfo.h<br>
+++ b/clang/lib/CodeGen/CGDebugInfo.h<br>
@@ -146,10 +146,6 @@ class CGDebugInfo {<br>
<br>
llvm::DenseMap<const char *, llvm::TrackingMDRef> DIFileCache;<br>
llvm::DenseMap<const FunctionDecl *, llvm::TrackingMDRef> SPCache;<br>
- /// Cache function definitions relevant to use for parameters mutation<br>
- /// analysis.<br>
- llvm::DenseMap<const FunctionDecl *, llvm::TrackingMDRef> SPDefCache;<br>
- llvm::DenseMap<const ParmVarDecl *, llvm::TrackingMDRef> ParamCache;<br>
/// Cache declarations relevant to DW_TAG_imported_declarations (C++<br>
/// using declarations) that aren't covered by other more specific caches.<br>
llvm::DenseMap<const Decl *, llvm::TrackingMDRef> DeclCache;<br>
<br>
diff --git a/clang/test/CodeGen/debug-info-param-modification.c b/clang/test/CodeGen/debug-info-param-modification.c<br>
deleted file mode 100644<br>
index f0a13a3777db..000000000000<br>
--- a/clang/test/CodeGen/debug-info-param-modification.c<br>
+++ /dev/null<br>
@@ -1,25 +0,0 @@<br>
-// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -Xclang -disable-llvm-passes -S -target x86_64-none-linux-gnu -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-ENTRY-VAL-OPT<br>
-// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -Xclang -disable-llvm-passes -S -target arm-none-linux-gnu -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-ENTRY-VAL-OPT<br>
-// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -Xclang -disable-llvm-passes -S -target aarch64-none-linux-gnu -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-ENTRY-VAL-OPT<br>
-// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -Xclang -disable-llvm-passes -S -target armeb-none-linux-gnu -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-ENTRY-VAL-OPT<br>
-<br>
-// CHECK-ENTRY-VAL-OPT: !DILocalVariable(name: "a", arg: 1, scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}})<br>
-// CHECK-ENTRY-VAL-OPT: !DILocalVariable(name: "b", arg: 2, scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}}, flags: DIFlagArgumentNotModified)<br>
-//<br>
-// For the os_log_helper:<br>
-// CHECK-ENTRY-VAL-OPT: !DILocalVariable(name: "buffer", arg: 1, {{.*}}, flags: DIFlagArtificial)<br>
-//<br>
-// RUN: %clang -g -O2 -Xclang -disable-llvm-passes -target x86_64-none-linux-gnu -S -emit-llvm %s -o - | FileCheck %s<br>
-// CHECK-NOT: !DILocalVariable(name: "b", arg: 2, scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}}, flags: DIFlagArgumentNotModified)<br>
-//<br>
-// For the os_log_helper:<br>
-// CHECK: !DILocalVariable(name: "buffer", arg: 1, {{.*}}, flags: DIFlagArtificial)<br>
-<br>
-int fn2 (int a, int b) {<br>
- ++a;<br>
- return b;<br>
-}<br>
-<br>
-void test_builtin_os_log(void *buf, int i, const char *data) {<br>
- __builtin_os_log_format(buf, "%d %{public}s %{private}.16P", i, data, data);<br>
-}<br>
<br>
diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py b/lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py<br>
index e0285e6d626d..1192c2b672f6 100644<br>
--- a/lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py<br>
+++ b/lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py<br>
@@ -6,7 +6,8 @@<br>
supported_platforms.extend(lldbplatformutil.getDarwinOSTriples())<br>
<br>
lldbinline.MakeInlineTest(__file__, globals(),<br>
- [decorators.skipUnlessPlatform(supported_platforms),<br>
+ [decorators.skipIf(bugnumber="<a href="http://llvm.org/pr44059" rel="noreferrer" target="_blank">llvm.org/pr44059</a>"),<br>
+ decorators.skipUnlessPlatform(supported_platforms),<br>
decorators.skipIf(compiler="clang", compiler_version=['<', '10.0']),<br>
decorators.skipUnlessArch('x86_64'),<br>
decorators.skipUnlessHasCallSiteInfo,<br>
<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div>