r181964 - Replace a pile of calls with an instance variable that's set
Eric Christopher
echristo at gmail.com
Mon May 20 13:00:36 PDT 2013
On Thu, May 16, 2013 at 11:55 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> 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.
>
I've gone ahead and just made it const for now out of likely laziness.
Thanks :)
-eric
>>
>>
>> -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
>> >
>> >
>
>
More information about the cfe-commits
mailing list