[clang] ce1f95a - Reland "[clang] Remove the DIFlagArgumentNotModified debug info flag"
Djordje Todorovic via cfe-commits
cfe-commits at lists.llvm.org
Sun Dec 29 22:14:03 PST 2019
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.todorovic at rt-rk.com <mailto:djordje.todorovic at 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 at lists.llvm.org <mailto:cfe-commits at lists.llvm.org> <mailto:cfe-commits at lists.llvm.org <mailto:cfe-commits at 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 at lists.llvm.org <mailto:cfe-commits at lists.llvm.org> <mailto:cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>>
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
>
More information about the cfe-commits
mailing list