Thanks! Best regards, Djordje
On 27.12.19. 20:30, David Blaikie wrote: > > > On Thu, Dec 26, 2019 at 11:58 PM Djordje Todorovic > <djordje.todoro...@rt-rk.com <mailto:djordje.todoro...@rt-rk.com>> wrote: > > Hi David, > > It's a good question. > > Current approach of the debug entry values will consider an entry value > as a valid value until the variable gets modified. > > Please consider this. > > void fn(int a) { > ... > a++; > } > > If there is an instruction that does not affect the code generated, e.g. > an ADD instruction that gets optimized out from the case above, it won't > force us to invalidate all the entry values before, since the instruction is > not there in the final code generated. The GCC does the same thing in that > situation. But if the instruction were at the beginning of the function (or > somewhere else), we believe that there is an DBG_VALUE representing that > variable's change (e.g. generated from the Salvage Debug Info), so the entry > value would not be used any more. > > If we come up with a case where a dead store causing an invalid use of > the entry values, that will be good point for improvements. > > > Ah, OK, so you actually want to know whether the entry value gets really > modified, makes sense to do that in the backend then - thanks for explaining! > > > > Best regards, > Djordje > > On 26.12.19. 22:33, David Blaikie wrote: > > > > > > On Wed, Nov 20, 2019 at 1:08 AM Djordje Todorovic via cfe-commits > <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> > <mailto:cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>>> > wrote: > > > > > > Author: Djordje Todorovic > > Date: 2019-11-20T10:08:07+01:00 > > New Revision: ce1f95a6e077693f93d8869245f911aff3eb7e4c > > > > URL: > https://github.com/llvm/llvm-project/commit/ce1f95a6e077693f93d8869245f911aff3eb7e4c > > DIFF: > https://github.com/llvm/llvm-project/commit/ce1f95a6e077693f93d8869245f911aff3eb7e4c.diff > > > > LOG: Reland "[clang] Remove the DIFlagArgumentNotModified debug > info flag" > > > > It turns out that the ExprMutationAnalyzer can be very slow when AST > > gets huge in some cases. The idea is to move this analysis to the > LLVM > > back-end level (more precisely, in the LiveDebugValues pass). The > new > > approach will remove the performance regression, simplify the > > implementation and give us front-end independent implementation. > > > > > > 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?) > > > > > > > > Differential Revision: https://reviews.llvm.org/D68206 > > > > Added: > > > > > > Modified: > > clang/lib/CodeGen/CGDebugInfo.cpp > > clang/lib/CodeGen/CGDebugInfo.h > > > lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py > > > > Removed: > > clang/test/CodeGen/debug-info-param-modification.c > > > > > > > ################################################################################ > > diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp > b/clang/lib/CodeGen/CGDebugInfo.cpp > > index 116517a9cb99..a9b3831aa0b5 100644 > > --- a/clang/lib/CodeGen/CGDebugInfo.cpp > > +++ b/clang/lib/CodeGen/CGDebugInfo.cpp > > @@ -18,7 +18,6 @@ > > #include "CodeGenFunction.h" > > #include "CodeGenModule.h" > > #include "ConstantEmitter.h" > > -#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h" > > #include "clang/AST/ASTContext.h" > > #include "clang/AST/DeclFriend.h" > > #include "clang/AST/DeclObjC.h" > > @@ -3686,15 +3685,6 @@ void > CGDebugInfo::EmitFunctionStart(GlobalDecl GD, SourceLocation Loc, > > if (HasDecl && isa<FunctionDecl>(D)) > > DeclCache[D->getCanonicalDecl()].reset(SP); > > > > - // We use the SPDefCache only in the case when the debug entry > values option > > - // is set, in order to speed up parameters modification analysis. > > - // > > - // FIXME: Use AbstractCallee here to support ObjCMethodDecl. > > - if (CGM.getCodeGenOpts().EnableDebugEntryValues && HasDecl) > > - if (auto *FD = dyn_cast<FunctionDecl>(D)) > > - if (FD->hasBody() && !FD->param_empty()) > > - SPDefCache[FD].reset(SP); > > - > > // Push the function onto the lexical block stack. > > LexicalBlockStack.emplace_back(SP); > > > > @@ -4097,11 +4087,6 @@ llvm::DILocalVariable > *CGDebugInfo::EmitDeclare(const VarDecl *VD, > > llvm::DebugLoc::get(Line, Column, Scope, > CurInlinedAt), > > Builder.GetInsertBlock()); > > > > - if (CGM.getCodeGenOpts().EnableDebugEntryValues && ArgNo) { > > - if (auto *PD = dyn_cast<ParmVarDecl>(VD)) > > - ParamCache[PD].reset(D); > > - } > > - > > return D; > > } > > > > @@ -4717,29 +4702,6 @@ void CGDebugInfo::setDwoId(uint64_t > Signature) { > > TheCU->setDWOId(Signature); > > } > > > > -/// Analyzes each function parameter to determine whether it is > constant > > -/// throughout the function body. > > -static void analyzeParametersModification( > > - ASTContext &Ctx, > > - llvm::DenseMap<const FunctionDecl *, llvm::TrackingMDRef> > &SPDefCache, > > - llvm::DenseMap<const ParmVarDecl *, llvm::TrackingMDRef> > &ParamCache) { > > - for (auto &SP : SPDefCache) { > > - auto *FD = SP.first; > > - assert(FD->hasBody() && "Functions must have body here"); > > - const Stmt *FuncBody = (*FD).getBody(); > > - for (auto Parm : FD->parameters()) { > > - ExprMutationAnalyzer FuncAnalyzer(*FuncBody, Ctx); > > - if (FuncAnalyzer.isMutated(Parm)) > > - continue; > > - > > - auto I = ParamCache.find(Parm); > > - assert(I != ParamCache.end() && "Parameters should be > already cached"); > > - auto *DIParm = cast<llvm::DILocalVariable>(I->second); > > - DIParm->setIsNotModified(); > > - } > > - } > > -} > > - > > void CGDebugInfo::finalize() { > > // Creating types might create further types - invalidating the > current > > // element and the size(), so don't cache/reference them. > > @@ -4812,10 +4774,6 @@ void CGDebugInfo::finalize() { > > if (auto MD = TypeCache[RT]) > > DBuilder.retainType(cast<llvm::DIType>(MD)); > > > > - if (CGM.getCodeGenOpts().EnableDebugEntryValues) > > - // This will be used to emit debug entry values. > > - analyzeParametersModification(CGM.getContext(), SPDefCache, > ParamCache); > > - > > DBuilder.finalize(); > > } > > > > > > diff --git a/clang/lib/CodeGen/CGDebugInfo.h > b/clang/lib/CodeGen/CGDebugInfo.h > > index 9a097615b4b4..5341bfa7f350 100644 > > --- a/clang/lib/CodeGen/CGDebugInfo.h > > +++ b/clang/lib/CodeGen/CGDebugInfo.h > > @@ -146,10 +146,6 @@ class CGDebugInfo { > > > > llvm::DenseMap<const char *, llvm::TrackingMDRef> DIFileCache; > > llvm::DenseMap<const FunctionDecl *, llvm::TrackingMDRef> > SPCache; > > - /// Cache function definitions relevant to use for parameters > mutation > > - /// analysis. > > - llvm::DenseMap<const FunctionDecl *, llvm::TrackingMDRef> > SPDefCache; > > - llvm::DenseMap<const ParmVarDecl *, llvm::TrackingMDRef> > ParamCache; > > /// Cache declarations relevant to DW_TAG_imported_declarations > (C++ > > /// using declarations) that aren't covered by other more > specific caches. > > llvm::DenseMap<const Decl *, llvm::TrackingMDRef> DeclCache; > > > > diff --git a/clang/test/CodeGen/debug-info-param-modification.c > b/clang/test/CodeGen/debug-info-param-modification.c > > deleted file mode 100644 > > index f0a13a3777db..000000000000 > > --- a/clang/test/CodeGen/debug-info-param-modification.c > > +++ /dev/null > > @@ -1,25 +0,0 @@ > > -// 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 > > -// 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 > > -// 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 > > -// 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 > > - > > -// CHECK-ENTRY-VAL-OPT: !DILocalVariable(name: "a", arg: 1, scope: > {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}}) > > -// CHECK-ENTRY-VAL-OPT: !DILocalVariable(name: "b", arg: 2, scope: > {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}}, flags: > DIFlagArgumentNotModified) > > -// > > -// For the os_log_helper: > > -// CHECK-ENTRY-VAL-OPT: !DILocalVariable(name: "buffer", arg: 1, > {{.*}}, flags: DIFlagArtificial) > > -// > > -// RUN: %clang -g -O2 -Xclang -disable-llvm-passes -target > x86_64-none-linux-gnu -S -emit-llvm %s -o - | FileCheck %s > > -// CHECK-NOT: !DILocalVariable(name: "b", arg: 2, scope: {{.*}}, > file: {{.*}}, line: {{.*}}, type: {{.*}}, flags: DIFlagArgumentNotModified) > > -// > > -// For the os_log_helper: > > -// CHECK: !DILocalVariable(name: "buffer", arg: 1, {{.*}}, flags: > DIFlagArtificial) > > - > > -int fn2 (int a, int b) { > > - ++a; > > - return b; > > -} > > - > > -void test_builtin_os_log(void *buf, int i, const char *data) { > > - __builtin_os_log_format(buf, "%d %{public}s %{private}.16P", i, > data, data); > > -} > > > > 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 > > index e0285e6d626d..1192c2b672f6 100644 > > --- > 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 > > @@ -6,7 +6,8 @@ > > supported_platforms.extend(lldbplatformutil.getDarwinOSTriples()) > > > > lldbinline.MakeInlineTest(__file__, globals(), > > - [decorators.skipUnlessPlatform(supported_platforms), > > + [decorators.skipIf(bugnumber="llvm.org/pr44059 > <http://llvm.org/pr44059> <http://llvm.org/pr44059>"), > > + decorators.skipUnlessPlatform(supported_platforms), > > decorators.skipIf(compiler="clang", > compiler_version=['<', '10.0']), > > decorators.skipUnlessArch('x86_64'), > > decorators.skipUnlessHasCallSiteInfo, > > > > > > > > _______________________________________________ > > cfe-commits mailing list > > cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> > <mailto:cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> > > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > > _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
