[clang] ce1f95a - Reland "[clang] Remove the DIFlagArgumentNotModified debug info flag"
David Blaikie via cfe-commits
cfe-commits at lists.llvm.org
Fri Dec 27 11:30:29 PST 2019
On Thu, Dec 26, 2019 at 11:58 PM Djordje Todorovic <
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>> 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>"),
> > + 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>
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191227/ea4cdc18/attachment-0001.html>
More information about the cfe-commits
mailing list