<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, May 15, 2013 at 6:00 PM, Eric Christopher <span dir="ltr"><<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Wed, May 15, 2013 at 5:53 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
><br>
><br>
><br>
> On Wed, May 15, 2013 at 5:45 PM, Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>><br>
> wrote:<br>
>><br>
>> Author: echristo<br>
>> Date: Wed May 15 19:45:23 2013<br>
>> New Revision: 181964<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=181964&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=181964&view=rev</a><br>
>> Log:<br>
>> Replace a pile of calls with an instance variable that's set<br>
>> once. Should be no functional change.<br>
>><br>
>> Modified:<br>
>> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp<br>
>> cfe/trunk/lib/CodeGen/CGDebugInfo.h<br>
>><br>
>> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=181964&r1=181963&r2=181964&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=181964&r1=181963&r2=181964&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)<br>
>> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Wed May 15 19:45:23 2013<br>
>> @@ -41,7 +41,8 @@ using namespace clang;<br>
>> using namespace clang::CodeGen;<br>
>><br>
>> CGDebugInfo::CGDebugInfo(CodeGenModule &CGM)<br>
>> - : CGM(CGM), DBuilder(CGM.getModule()),<br>
>> + : CGM(CGM), DebugKind(CGM.getCodeGenOpts().getDebugInfo()),<br>
><br>
><br>
> Duplicating information always makes me concerned that it might end up<br>
> getting out of sync - is this a hypothetical perf improvement, or just a<br>
> syntactic reduction - if it's the latter, perhaps a private member utility<br>
> function might suffice? Short of that I might suggest making the member<br>
> 'const' but I realize that's a bit uncommon (so I'd err towards the former,<br>
> generally).<br>
><br>
<br>
</div></div>Little bit of both. I'd originally made this an inline function and<br>
then made it a member variable when I realized that the option had<br>
better not change during compilation and we call a lot of those<br>
routines frequently - but they're mostly in asserts anyhow so any real<br>
performance gain would be largely theoretical. However, 'const' is<br>
fine with me as well. :)<br>
<br>
Thoughts?<br></blockquote><div><br></div><div style>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.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
-eric<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
>><br>
>> + DBuilder(CGM.getModule()),<br>
>> BlockLiteralGenericSet(false) {<br>
>> CreateCompileUnit();<br>
>> }<br>
>> @@ -602,7 +603,7 @@ llvm::DIDescriptor CGDebugInfo::createCo<br>
>> /// then emit record's fwd if debug info size reduction is enabled.<br>
>> llvm::DIType CGDebugInfo::CreatePointeeType(QualType PointeeTy,<br>
>> llvm::DIFile Unit) {<br>
>> - if (CGM.getCodeGenOpts().getDebugInfo() !=<br>
>> CodeGenOptions::LimitedDebugInfo)<br>
>> + if (DebugKind != CodeGenOptions::LimitedDebugInfo)<br>
>> return getOrCreateType(PointeeTy, Unit);<br>
>><br>
>> // Limit debug info for the pointee type.<br>
>> @@ -1367,7 +1368,7 @@ CollectVTableInfo(const CXXRecordDecl *R<br>
>> /// getOrCreateRecordType - Emit record type's standalone debug info.<br>
>> llvm::DIType CGDebugInfo::getOrCreateRecordType(QualType RTy,<br>
>> SourceLocation Loc) {<br>
>> - assert(CGM.getCodeGenOpts().getDebugInfo() >=<br>
>> CodeGenOptions::LimitedDebugInfo);<br>
>> + assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);<br>
>> llvm::DIType T = getOrCreateType(RTy, getOrCreateFile(Loc));<br>
>> return T;<br>
>> }<br>
>> @@ -1376,7 +1377,7 @@ llvm::DIType CGDebugInfo::getOrCreateRec<br>
>> /// debug info.<br>
>> llvm::DIType CGDebugInfo::getOrCreateInterfaceType(QualType D,<br>
>> SourceLocation Loc) {<br>
>> - assert(CGM.getCodeGenOpts().getDebugInfo() >=<br>
>> CodeGenOptions::LimitedDebugInfo);<br>
>> + assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);<br>
>> llvm::DIType T = getOrCreateType(D, getOrCreateFile(Loc));<br>
>> RetainedTypes.push_back(D.getAsOpaquePtr());<br>
>> return T;<br>
>> @@ -2083,7 +2084,7 @@ llvm::DIType CGDebugInfo::CreateLimitedT<br>
>> StringRef RDName = getClassName(RD);<br>
>><br>
>> llvm::DIDescriptor RDContext;<br>
>> - if (CGM.getCodeGenOpts().getDebugInfo() ==<br>
>> CodeGenOptions::LimitedDebugInfo)<br>
>> + if (DebugKind == CodeGenOptions::LimitedDebugInfo)<br>
>> RDContext = createContextChain(cast<Decl>(RD->getDeclContext()));<br>
>> else<br>
>> RDContext = getContextDescriptor(cast<Decl>(RD->getDeclContext()));<br>
>> @@ -2293,10 +2294,10 @@ void CGDebugInfo::EmitFunctionStart(Glob<br>
>> if (LinkageName == Name ||<br>
>> (!CGM.getCodeGenOpts().EmitGcovArcs &&<br>
>> !CGM.getCodeGenOpts().EmitGcovNotes &&<br>
>> - CGM.getCodeGenOpts().getDebugInfo() <=<br>
>> CodeGenOptions::DebugLineTablesOnly))<br>
>> + DebugKind <= CodeGenOptions::DebugLineTablesOnly))<br>
>> LinkageName = StringRef();<br>
>><br>
>> - if (CGM.getCodeGenOpts().getDebugInfo() >=<br>
>> CodeGenOptions::LimitedDebugInfo) {<br>
>> + if (DebugKind >= CodeGenOptions::LimitedDebugInfo) {<br>
>> if (const NamespaceDecl *NSDecl =<br>
>> dyn_cast_or_null<NamespaceDecl>(FD->getDeclContext()))<br>
>> FDContext = getOrCreateNameSpace(NSDecl);<br>
>> @@ -2325,7 +2326,7 @@ void CGDebugInfo::EmitFunctionStart(Glob<br>
>> llvm::DIType DIFnType;<br>
>> llvm::DISubprogram SPDecl;<br>
>> if (HasDecl &&<br>
>> - CGM.getCodeGenOpts().getDebugInfo() >=<br>
>> CodeGenOptions::LimitedDebugInfo) {<br>
>> + DebugKind >= CodeGenOptions::LimitedDebugInfo) {<br>
>> DIFnType = getOrCreateFunctionType(D, FnType, Unit);<br>
>> SPDecl = getFunctionDeclaration(D);<br>
>> } else {<br>
>> @@ -2514,7 +2515,7 @@ llvm::DIType CGDebugInfo::EmitTypeForVar<br>
>> void CGDebugInfo::EmitDeclare(const VarDecl *VD, unsigned Tag,<br>
>> llvm::Value *Storage,<br>
>> unsigned ArgNo, CGBuilderTy &Builder) {<br>
>> - assert(CGM.getCodeGenOpts().getDebugInfo() >=<br>
>> CodeGenOptions::LimitedDebugInfo);<br>
>> + assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);<br>
>> assert(!LexicalBlockStack.empty() && "Region stack mismatch, stack<br>
>> empty!");<br>
>><br>
>> llvm::DIFile Unit = getOrCreateFile(VD->getLocation());<br>
>> @@ -2654,7 +2655,7 @@ void CGDebugInfo::EmitDeclare(const VarD<br>
>> void CGDebugInfo::EmitDeclareOfAutoVariable(const VarDecl *VD,<br>
>> llvm::Value *Storage,<br>
>> CGBuilderTy &Builder) {<br>
>> - assert(CGM.getCodeGenOpts().getDebugInfo() >=<br>
>> CodeGenOptions::LimitedDebugInfo);<br>
>> + assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);<br>
>> EmitDeclare(VD, llvm::dwarf::DW_TAG_auto_variable, Storage, 0,<br>
>> Builder);<br>
>> }<br>
>><br>
>> @@ -2675,7 +2676,7 @@ void CGDebugInfo::EmitDeclareOfBlockDecl<br>
>> llvm::Value *Storage,<br>
>> CGBuilderTy &Builder,<br>
>> const CGBlockInfo<br>
>> &blockInfo) {<br>
>> - assert(CGM.getCodeGenOpts().getDebugInfo() >=<br>
>> CodeGenOptions::LimitedDebugInfo);<br>
>> + assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);<br>
>> assert(!LexicalBlockStack.empty() && "Region stack mismatch, stack<br>
>> empty!");<br>
>><br>
>> if (Builder.GetInsertBlock() == 0)<br>
>> @@ -2744,7 +2745,7 @@ void CGDebugInfo::EmitDeclareOfBlockDecl<br>
>> void CGDebugInfo::EmitDeclareOfArgVariable(const VarDecl *VD, llvm::Value<br>
>> *AI,<br>
>> unsigned ArgNo,<br>
>> CGBuilderTy &Builder) {<br>
>> - assert(CGM.getCodeGenOpts().getDebugInfo() >=<br>
>> CodeGenOptions::LimitedDebugInfo);<br>
>> + assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);<br>
>> EmitDeclare(VD, llvm::dwarf::DW_TAG_arg_variable, AI, ArgNo, Builder);<br>
>> }<br>
>><br>
>> @@ -2762,7 +2763,7 @@ void CGDebugInfo::EmitDeclareOfBlockLite<br>
>> llvm::Value *Arg,<br>
>> llvm::Value<br>
>> *LocalAddr,<br>
>> CGBuilderTy<br>
>> &Builder) {<br>
>> - assert(CGM.getCodeGenOpts().getDebugInfo() >=<br>
>> CodeGenOptions::LimitedDebugInfo);<br>
>> + assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);<br>
>> ASTContext &C = CGM.getContext();<br>
>> const BlockDecl *blockDecl = block.getBlockDecl();<br>
>><br>
>> @@ -2928,7 +2929,7 @@ llvm::DIDerivedType CGDebugInfo::getStat<br>
>> /// EmitGlobalVariable - Emit information about a global variable.<br>
>> void CGDebugInfo::EmitGlobalVariable(llvm::GlobalVariable *Var,<br>
>> const VarDecl *D) {<br>
>> - assert(CGM.getCodeGenOpts().getDebugInfo() >=<br>
>> CodeGenOptions::LimitedDebugInfo);<br>
>> + assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);<br>
>> // Create global variable debug descriptor.<br>
>> llvm::DIFile Unit = getOrCreateFile(D->getLocation());<br>
>> unsigned LineNo = getLineNumber(D->getLocation());<br>
>> @@ -2963,7 +2964,7 @@ void CGDebugInfo::EmitGlobalVariable(llv<br>
>> /// EmitGlobalVariable - Emit information about an objective-c interface.<br>
>> void CGDebugInfo::EmitGlobalVariable(llvm::GlobalVariable *Var,<br>
>> ObjCInterfaceDecl *ID) {<br>
>> - assert(CGM.getCodeGenOpts().getDebugInfo() >=<br>
>> CodeGenOptions::LimitedDebugInfo);<br>
>> + assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);<br>
>> // Create global variable debug descriptor.<br>
>> llvm::DIFile Unit = getOrCreateFile(ID->getLocation());<br>
>> unsigned LineNo = getLineNumber(ID->getLocation());<br>
>> @@ -2989,7 +2990,7 @@ void CGDebugInfo::EmitGlobalVariable(llv<br>
>> /// EmitGlobalVariable - Emit global variable's debug info.<br>
>> void CGDebugInfo::EmitGlobalVariable(const ValueDecl *VD,<br>
>> llvm::Constant *Init) {<br>
>> - assert(CGM.getCodeGenOpts().getDebugInfo() >=<br>
>> CodeGenOptions::LimitedDebugInfo);<br>
>> + assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);<br>
>> // Create the descriptor for the variable.<br>
>> llvm::DIFile Unit = getOrCreateFile(VD->getLocation());<br>
>> StringRef Name = VD->getName();<br>
>><br>
>> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.h<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=181964&r1=181963&r2=181964&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=181964&r1=181963&r2=181964&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- cfe/trunk/lib/CodeGen/CGDebugInfo.h (original)<br>
>> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.h Wed May 15 19:45:23 2013<br>
>> @@ -18,6 +18,7 @@<br>
>> #include "clang/AST/Expr.h"<br>
>> #include "clang/AST/Type.h"<br>
>> #include "clang/Basic/SourceLocation.h"<br>
>> +#include "clang/Frontend/CodeGenOptions.h"<br>
>> #include "llvm/ADT/DenseMap.h"<br>
>> #include "llvm/DIBuilder.h"<br>
>> #include "llvm/DebugInfo.h"<br>
>> @@ -46,6 +47,7 @@ namespace CodeGen {<br>
>> /// the backend.<br>
>> class CGDebugInfo {<br>
>> CodeGenModule &CGM;<br>
>> + CodeGenOptions::DebugInfoKind DebugKind;<br>
>> llvm::DIBuilder DBuilder;<br>
>> llvm::DICompileUnit TheCU;<br>
>> SourceLocation CurLoc, PrevLoc;<br>
>> @@ -274,7 +276,7 @@ public:<br>
>> /// getOrCreateInterfaceType - Emit an objective c interface type<br>
>> standalone<br>
>> /// debug info.<br>
>> llvm::DIType getOrCreateInterfaceType(QualType Ty,<br>
>> - SourceLocation Loc);<br>
>> + SourceLocation Loc);<br>
>><br>
>> private:<br>
>> /// EmitDeclare - Emit call to llvm.dbg.declare for a variable<br>
>> declaration.<br>
>><br>
>><br>
>> _______________________________________________<br>
>> cfe-commits mailing list<br>
>> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
><br>
><br>
</div></div></blockquote></div><br></div></div>