r181964 - Replace a pile of calls with an instance variable that's set
David Blaikie
dblaikie at gmail.com
Thu May 16 11:55:22 PDT 2013
On Wed, May 15, 2013 at 6:00 PM, Eric Christopher <echristo at gmail.com>wrote:
> 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=181964&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 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?
>
While, yes, these values should never change even if copied, it'd just be
one less possibility/cognitive load to add by not duplicating the variable
- so I would tend towards that direction, but not by any great margin. If
you think the possible theoretical perf improvement might be worthwhile
compared to that minor cognitive load, then making it 'const' will suffice.
>
> -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=181964&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
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130516/9fd4a472/attachment.html>
More information about the cfe-commits
mailing list