r181964 - Replace a pile of calls with an instance variable that's set

Robinson, Paul Paul_Robinson at playstation.sony.com
Thu May 16 11:49:29 PDT 2013


> -----Original Message-----
> From: cfe-commits-bounces at cs.uiuc.edu [mailto:cfe-commits-
> bounces at cs.uiuc.edu] On Behalf Of Eric Christopher
> Sent: Wednesday, May 15, 2013 6:00 PM
> To: David Blaikie
> Cc: llvm cfe
> Subject: Re: r181964 - Replace a pile of calls with an instance variable
> that's set
> 
> On Wed, May 15, 2013 at 5:53 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >
> >
> >
> > On Wed, May 15, 2013 at 5:45 PM, Eric Christopher <echristo at gmail.com>
> > wrote:
> >>
> >> Author: echristo
> >> Date: Wed May 15 19:45:23 2013
> >> New Revision: 181964
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=181964&view=rev
> >> Log:
> >> Replace a pile of calls with an instance variable that's set
> >> once. Should be no functional change.
> >>
> >> Modified:
> >>     cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> >>     cfe/trunk/lib/CodeGen/CGDebugInfo.h
> >>
> >> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> >> URL:
> >> http://llvm.org/viewvc/llvm-
> project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=181964&r1=181963&r2=18
> 1964&view=diff
> >>
> >>
> ========================================================================
> ======
> >> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> >> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Wed May 15 19:45:23 2013
> >> @@ -41,7 +41,8 @@ using namespace clang;
> >>  using namespace clang::CodeGen;
> >>
> >>  CGDebugInfo::CGDebugInfo(CodeGenModule &CGM)
> >> -  : CGM(CGM), DBuilder(CGM.getModule()),
> >> +  : CGM(CGM), DebugKind(CGM.getCodeGenOpts().getDebugInfo()),
> >
> >
> > Duplicating information always makes me concerned that it might end up
> > getting out of sync - is this a hypothetical perf improvement, or just
> a
> > syntactic reduction - if it's the latter, perhaps a private member
> utility
> > function might suffice? Short of that I might suggest making the
> member
> > 'const' but I realize that's a bit uncommon (so I'd err towards the
> former,
> > generally).
> >
> 
> Little bit of both. I'd originally made this an inline function and
> then made it a member variable when I realized that the option had
> better not change during compilation

Amen.

> and we call a lot of those
> routines frequently - but they're mostly in asserts anyhow so any real
> performance gain would be largely theoretical. However, 'const' is
> fine with me as well. :)
> 
> Thoughts?

+1.
--paulr

> 
> -eric
> 
> >>
> >> +    DBuilder(CGM.getModule()),
> >>      BlockLiteralGenericSet(false) {
> >>    CreateCompileUnit();
> >>  }
> >> @@ -602,7 +603,7 @@ llvm::DIDescriptor CGDebugInfo::createCo
> >>  /// then emit record's fwd if debug info size reduction is enabled.
> >>  llvm::DIType CGDebugInfo::CreatePointeeType(QualType PointeeTy,
> >>                                              llvm::DIFile Unit) {
> >> -  if (CGM.getCodeGenOpts().getDebugInfo() !=
> >> CodeGenOptions::LimitedDebugInfo)
> >> +  if (DebugKind != CodeGenOptions::LimitedDebugInfo)
> >>      return getOrCreateType(PointeeTy, Unit);
> >>
> >>    // Limit debug info for the pointee type.
> >> @@ -1367,7 +1368,7 @@ CollectVTableInfo(const CXXRecordDecl *R
> >>  /// getOrCreateRecordType - Emit record type's standalone debug
> info.
> >>  llvm::DIType CGDebugInfo::getOrCreateRecordType(QualType RTy,
> >>                                                  SourceLocation Loc)
> {
> >> -  assert(CGM.getCodeGenOpts().getDebugInfo() >=
> >> CodeGenOptions::LimitedDebugInfo);
> >> +  assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);
> >>    llvm::DIType T = getOrCreateType(RTy, getOrCreateFile(Loc));
> >>    return T;
> >>  }
> >> @@ -1376,7 +1377,7 @@ llvm::DIType CGDebugInfo::getOrCreateRec
> >>  /// debug info.
> >>  llvm::DIType CGDebugInfo::getOrCreateInterfaceType(QualType D,
> >>                                                     SourceLocation
> Loc) {
> >> -  assert(CGM.getCodeGenOpts().getDebugInfo() >=
> >> CodeGenOptions::LimitedDebugInfo);
> >> +  assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);
> >>    llvm::DIType T = getOrCreateType(D, getOrCreateFile(Loc));
> >>    RetainedTypes.push_back(D.getAsOpaquePtr());
> >>    return T;
> >> @@ -2083,7 +2084,7 @@ llvm::DIType CGDebugInfo::CreateLimitedT
> >>    StringRef RDName = getClassName(RD);
> >>
> >>    llvm::DIDescriptor RDContext;
> >> -  if (CGM.getCodeGenOpts().getDebugInfo() ==
> >> CodeGenOptions::LimitedDebugInfo)
> >> +  if (DebugKind == CodeGenOptions::LimitedDebugInfo)
> >>      RDContext = createContextChain(cast<Decl>(RD-
> >getDeclContext()));
> >>    else
> >>      RDContext = getContextDescriptor(cast<Decl>(RD-
> >getDeclContext()));
> >> @@ -2293,10 +2294,10 @@ void CGDebugInfo::EmitFunctionStart(Glob
> >>      if (LinkageName == Name ||
> >>          (!CGM.getCodeGenOpts().EmitGcovArcs &&
> >>           !CGM.getCodeGenOpts().EmitGcovNotes &&
> >> -         CGM.getCodeGenOpts().getDebugInfo() <=
> >> CodeGenOptions::DebugLineTablesOnly))
> >> +         DebugKind <= CodeGenOptions::DebugLineTablesOnly))
> >>        LinkageName = StringRef();
> >>
> >> -    if (CGM.getCodeGenOpts().getDebugInfo() >=
> >> CodeGenOptions::LimitedDebugInfo) {
> >> +    if (DebugKind >= CodeGenOptions::LimitedDebugInfo) {
> >>        if (const NamespaceDecl *NSDecl =
> >>            dyn_cast_or_null<NamespaceDecl>(FD->getDeclContext()))
> >>          FDContext = getOrCreateNameSpace(NSDecl);
> >> @@ -2325,7 +2326,7 @@ void CGDebugInfo::EmitFunctionStart(Glob
> >>    llvm::DIType DIFnType;
> >>    llvm::DISubprogram SPDecl;
> >>    if (HasDecl &&
> >> -      CGM.getCodeGenOpts().getDebugInfo() >=
> >> CodeGenOptions::LimitedDebugInfo) {
> >> +      DebugKind >= CodeGenOptions::LimitedDebugInfo) {
> >>      DIFnType = getOrCreateFunctionType(D, FnType, Unit);
> >>      SPDecl = getFunctionDeclaration(D);
> >>    } else {
> >> @@ -2514,7 +2515,7 @@ llvm::DIType CGDebugInfo::EmitTypeForVar
> >>  void CGDebugInfo::EmitDeclare(const VarDecl *VD, unsigned Tag,
> >>                                llvm::Value *Storage,
> >>                                unsigned ArgNo, CGBuilderTy &Builder)
> {
> >> -  assert(CGM.getCodeGenOpts().getDebugInfo() >=
> >> CodeGenOptions::LimitedDebugInfo);
> >> +  assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);
> >>    assert(!LexicalBlockStack.empty() && "Region stack mismatch, stack
> >> empty!");
> >>
> >>    llvm::DIFile Unit = getOrCreateFile(VD->getLocation());
> >> @@ -2654,7 +2655,7 @@ void CGDebugInfo::EmitDeclare(const VarD
> >>  void CGDebugInfo::EmitDeclareOfAutoVariable(const VarDecl *VD,
> >>                                              llvm::Value *Storage,
> >>                                              CGBuilderTy &Builder) {
> >> -  assert(CGM.getCodeGenOpts().getDebugInfo() >=
> >> CodeGenOptions::LimitedDebugInfo);
> >> +  assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);
> >>    EmitDeclare(VD, llvm::dwarf::DW_TAG_auto_variable, Storage, 0,
> >> Builder);
> >>  }
> >>
> >> @@ -2675,7 +2676,7 @@ void CGDebugInfo::EmitDeclareOfBlockDecl
> >>                                                      llvm::Value
> *Storage,
> >>                                                      CGBuilderTy
> &Builder,
> >>                                                   const CGBlockInfo
> >> &blockInfo) {
> >> -  assert(CGM.getCodeGenOpts().getDebugInfo() >=
> >> CodeGenOptions::LimitedDebugInfo);
> >> +  assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);
> >>    assert(!LexicalBlockStack.empty() && "Region stack mismatch, stack
> >> empty!");
> >>
> >>    if (Builder.GetInsertBlock() == 0)
> >> @@ -2744,7 +2745,7 @@ void CGDebugInfo::EmitDeclareOfBlockDecl
> >>  void CGDebugInfo::EmitDeclareOfArgVariable(const VarDecl *VD,
> llvm::Value
> >> *AI,
> >>                                             unsigned ArgNo,
> >>                                             CGBuilderTy &Builder) {
> >> -  assert(CGM.getCodeGenOpts().getDebugInfo() >=
> >> CodeGenOptions::LimitedDebugInfo);
> >> +  assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);
> >>    EmitDeclare(VD, llvm::dwarf::DW_TAG_arg_variable, AI, ArgNo,
> Builder);
> >>  }
> >>
> >> @@ -2762,7 +2763,7 @@ void CGDebugInfo::EmitDeclareOfBlockLite
> >>                                                         llvm::Value
> *Arg,
> >>                                                         llvm::Value
> >> *LocalAddr,
> >>                                                         CGBuilderTy
> >> &Builder) {
> >> -  assert(CGM.getCodeGenOpts().getDebugInfo() >=
> >> CodeGenOptions::LimitedDebugInfo);
> >> +  assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);
> >>    ASTContext &C = CGM.getContext();
> >>    const BlockDecl *blockDecl = block.getBlockDecl();
> >>
> >> @@ -2928,7 +2929,7 @@ llvm::DIDerivedType CGDebugInfo::getStat
> >>  /// EmitGlobalVariable - Emit information about a global variable.
> >>  void CGDebugInfo::EmitGlobalVariable(llvm::GlobalVariable *Var,
> >>                                       const VarDecl *D) {
> >> -  assert(CGM.getCodeGenOpts().getDebugInfo() >=
> >> CodeGenOptions::LimitedDebugInfo);
> >> +  assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);
> >>    // Create global variable debug descriptor.
> >>    llvm::DIFile Unit = getOrCreateFile(D->getLocation());
> >>    unsigned LineNo = getLineNumber(D->getLocation());
> >> @@ -2963,7 +2964,7 @@ void CGDebugInfo::EmitGlobalVariable(llv
> >>  /// EmitGlobalVariable - Emit information about an objective-c
> interface.
> >>  void CGDebugInfo::EmitGlobalVariable(llvm::GlobalVariable *Var,
> >>                                       ObjCInterfaceDecl *ID) {
> >> -  assert(CGM.getCodeGenOpts().getDebugInfo() >=
> >> CodeGenOptions::LimitedDebugInfo);
> >> +  assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);
> >>    // Create global variable debug descriptor.
> >>    llvm::DIFile Unit = getOrCreateFile(ID->getLocation());
> >>    unsigned LineNo = getLineNumber(ID->getLocation());
> >> @@ -2989,7 +2990,7 @@ void CGDebugInfo::EmitGlobalVariable(llv
> >>  /// EmitGlobalVariable - Emit global variable's debug info.
> >>  void CGDebugInfo::EmitGlobalVariable(const ValueDecl *VD,
> >>                                       llvm::Constant *Init) {
> >> -  assert(CGM.getCodeGenOpts().getDebugInfo() >=
> >> CodeGenOptions::LimitedDebugInfo);
> >> +  assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);
> >>    // Create the descriptor for the variable.
> >>    llvm::DIFile Unit = getOrCreateFile(VD->getLocation());
> >>    StringRef Name = VD->getName();
> >>
> >> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.h
> >> URL:
> >> http://llvm.org/viewvc/llvm-
> project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=181964&r1=181963&r2=1819
> 64&view=diff
> >>
> >>
> ========================================================================
> ======
> >> --- cfe/trunk/lib/CodeGen/CGDebugInfo.h (original)
> >> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.h Wed May 15 19:45:23 2013
> >> @@ -18,6 +18,7 @@
> >>  #include "clang/AST/Expr.h"
> >>  #include "clang/AST/Type.h"
> >>  #include "clang/Basic/SourceLocation.h"
> >> +#include "clang/Frontend/CodeGenOptions.h"
> >>  #include "llvm/ADT/DenseMap.h"
> >>  #include "llvm/DIBuilder.h"
> >>  #include "llvm/DebugInfo.h"
> >> @@ -46,6 +47,7 @@ namespace CodeGen {
> >>  /// the backend.
> >>  class CGDebugInfo {
> >>    CodeGenModule &CGM;
> >> +  CodeGenOptions::DebugInfoKind DebugKind;
> >>    llvm::DIBuilder DBuilder;
> >>    llvm::DICompileUnit TheCU;
> >>    SourceLocation CurLoc, PrevLoc;
> >> @@ -274,7 +276,7 @@ public:
> >>    /// getOrCreateInterfaceType - Emit an objective c interface type
> >> standalone
> >>    /// debug info.
> >>    llvm::DIType getOrCreateInterfaceType(QualType Ty,
> >> -                                       SourceLocation Loc);
> >> +                                        SourceLocation Loc);
> >>
> >>  private:
> >>    /// EmitDeclare - Emit call to llvm.dbg.declare for a variable
> >> declaration.
> >>
> >>
> >> _______________________________________________
> >> cfe-commits mailing list
> >> cfe-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >
> >
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits





More information about the cfe-commits mailing list