<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Jul 29, 2014 at 3:25 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="">On Tue, Jul 29, 2014 at 11:20 AM, Manman Ren <<a href="mailto:manman.ren@gmail.com">manman.ren@gmail.com</a>> wrote:<br>
> Author: mren<br>
> Date: Tue Jul 29 13:20:39 2014<br>
> New Revision: 214189<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=214189&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=214189&view=rev</a><br>
> Log:<br>
> [Debug Info] remove DITrivialType and use null to represent unspecified param.<br>
><br>
> Per feedback on r214111, we are going to use null to represent unspecified<br>
> parameter. If the type array is {null}, it means a function that returns void;<br>
> If the type array is {null, null}, it means a variadic function that returns<br>
> void. In summary if we have more than one element in the type array and the last<br>
> element is null, it is a variadic function.<br>
<br>
</div>For future reference, this sort of thing (where not much or nothing<br>
from the previous patch is kept) is often easier to review by<br>
reverting the previous patch and committing a new one.<br></blockquote><div><br></div><div><br></div><div>Yeah, I thought about that, but reverting the previous patch will break the build, so I combined the two patches together.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><div class="h5"><br>
><br>
> rdar://17628609<br>
><br>
> Modified:<br>
> llvm/trunk/include/llvm/IR/DIBuilder.h<br>
> llvm/trunk/include/llvm/IR/DebugInfo.h<br>
> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp<br>
> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp<br>
> llvm/trunk/lib/IR/DIBuilder.cpp<br>
> llvm/trunk/lib/IR/DebugInfo.cpp<br>
> llvm/trunk/test/DebugInfo/varargs.ll<br>
><br>
> Modified: llvm/trunk/include/llvm/IR/DIBuilder.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DIBuilder.h?rev=214189&r1=214188&r2=214189&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DIBuilder.h?rev=214189&r1=214188&r2=214189&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/include/llvm/IR/DIBuilder.h (original)<br>
> +++ llvm/trunk/include/llvm/IR/DIBuilder.h Tue Jul 29 13:20:39 2014<br>
> @@ -465,7 +465,7 @@ namespace llvm {<br>
><br>
> /// createUnspecifiedParameter - Create unspecified parameter type<br>
> /// for a subroutine type.<br>
> - DITrivialType createUnspecifiedParameter();<br>
> + DIBasicType createUnspecifiedParameter();<br>
><br>
> /// getOrCreateArray - Get a DIArray, create one if required.<br>
> DIArray getOrCreateArray(ArrayRef<Value *> Elements);<br>
><br>
> Modified: llvm/trunk/include/llvm/IR/DebugInfo.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DebugInfo.h?rev=214189&r1=214188&r2=214189&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DebugInfo.h?rev=214189&r1=214188&r2=214189&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/include/llvm/IR/DebugInfo.h (original)<br>
> +++ llvm/trunk/include/llvm/IR/DebugInfo.h Tue Jul 29 13:20:39 2014<br>
> @@ -129,7 +129,6 @@ public:<br>
> bool isCompositeType() const;<br>
> bool isSubroutineType() const;<br>
> bool isBasicType() const;<br>
> - bool isTrivialType() const;<br>
> bool isVariable() const;<br>
> bool isSubprogram() const;<br>
> bool isGlobalVariable() const;<br>
> @@ -142,7 +141,6 @@ public:<br>
> bool isSubrange() const;<br>
> bool isEnumerator() const;<br>
> bool isType() const;<br>
> - bool isUnspecifiedParameter() const;<br>
> bool isTemplateTypeParameter() const;<br>
> bool isTemplateValueParameter() const;<br>
> bool isObjCProperty() const;<br>
> @@ -308,36 +306,15 @@ public:<br>
> /// Verify - Verify that a type descriptor is well formed.<br>
> bool Verify() const;<br>
><br>
> - DIScopeRef getContext() const {<br>
> - assert(!isTrivialType() && "no context for DITrivialType");<br>
> - return getFieldAs<DIScopeRef>(2);<br>
> - }<br>
> - StringRef getName() const {<br>
> - assert(!isTrivialType() && "no name for DITrivialType");<br>
> - return getStringField(3);<br>
> - }<br>
> - unsigned getLineNumber() const {<br>
> - assert(!isTrivialType() && "no line number for DITrivialType");<br>
> - return getUnsignedField(4);<br>
> - }<br>
> - uint64_t getSizeInBits() const {<br>
> - assert(!isTrivialType() && "no SizeInBits for DITrivialType");<br>
> - return getUInt64Field(5);<br>
> - }<br>
> - uint64_t getAlignInBits() const {<br>
> - assert(!isTrivialType() && "no AlignInBits for DITrivialType");<br>
> - return getUInt64Field(6);<br>
> - }<br>
> + DIScopeRef getContext() const { return getFieldAs<DIScopeRef>(2); }<br>
> + StringRef getName() const { return getStringField(3); }<br>
> + unsigned getLineNumber() const { return getUnsignedField(4); }<br>
> + uint64_t getSizeInBits() const { return getUInt64Field(5); }<br>
> + uint64_t getAlignInBits() const { return getUInt64Field(6); }<br>
> // FIXME: Offset is only used for DW_TAG_member nodes. Making every type<br>
> // carry this is just plain insane.<br>
> - uint64_t getOffsetInBits() const {<br>
> - assert(!isTrivialType() && "no OffsetInBits for DITrivialType");<br>
> - return getUInt64Field(7);<br>
> - }<br>
> - unsigned getFlags() const {<br>
> - assert(!isTrivialType() && "no flag for DITrivialType");<br>
> - return getUnsignedField(8);<br>
> - }<br>
> + uint64_t getOffsetInBits() const { return getUInt64Field(7); }<br>
> + unsigned getFlags() const { return getUnsignedField(8); }<br>
> bool isPrivate() const { return (getFlags() & FlagPrivate) != 0; }<br>
> bool isProtected() const { return (getFlags() & FlagProtected) != 0; }<br>
> bool isForwardDecl() const { return (getFlags() & FlagFwdDecl) != 0; }<br>
> @@ -370,12 +347,6 @@ public:<br>
> void replaceAllUsesWith(MDNode *D);<br>
> };<br>
><br>
> -class DITrivialType : public DIType {<br>
> -public:<br>
> - explicit DITrivialType(const MDNode *N = nullptr) : DIType(N) {}<br>
> - bool Verify() const;<br>
> -};<br>
> -<br>
> /// DIBasicType - A basic type, like 'int' or 'float'.<br>
> class DIBasicType : public DIType {<br>
> public:<br>
><br>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=214189&r1=214188&r2=214189&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=214189&r1=214188&r2=214189&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)<br>
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Tue Jul 29 13:20:39 2014<br>
> @@ -469,11 +469,13 @@ DIE *DwarfDebug::createScopeChildrenDIE(<br>
> // If this is a variadic function, add an unspecified parameter.<br>
> DISubprogram SP(Scope->getScopeNode());<br>
> DITypeArray FnArgs = SP.getType().getTypeArray();<br>
> - if (resolve(FnArgs.getElement(FnArgs.getNumElements() - 1))<br>
> - .isUnspecifiedParameter()) {<br>
> + // If we have a single element of null, it is a function that returns void.<br>
> + // If we have more than one elements and the last one is null, it is a<br>
> + // variadic function.<br>
> + if (FnArgs.getNumElements() > 1 &&<br>
> + !resolve(FnArgs.getElement(FnArgs.getNumElements() - 1)))<br>
<br>
</div></div>I don't think there's a need for the resolve call here, since it's<br>
just a null check.<br></blockquote><div><br></div><div>In r214240,</div><div><br></div><div>Thanks,</div><div>Manman</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class=""><br>
> Children.push_back(<br>
> make_unique<DIE>(dwarf::DW_TAG_unspecified_parameters));<br>
> - }<br>
> }<br>
><br>
> // Collect lexical scope children first.<br>
><br>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp?rev=214189&r1=214188&r2=214189&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp?rev=214189&r1=214188&r2=214189&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (original)<br>
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp Tue Jul 29 13:20:39 2014<br>
> @@ -1132,7 +1132,7 @@ void DwarfUnit::constructTypeDIE(DIE &Bu<br>
> void DwarfUnit::constructSubprogramArguments(DIE &Buffer, DITypeArray Args) {<br>
> for (unsigned i = 1, N = Args.getNumElements(); i < N; ++i) {<br>
> DIType Ty = resolve(Args.getElement(i));<br>
> - if (Ty.isUnspecifiedParameter()) {<br>
> + if (!Ty) {<br>
> assert(i == N-1 && "Unspecified parameter must be the last argument");<br>
> createAndAddDIE(dwarf::DW_TAG_unspecified_parameters, Buffer);<br>
> } else {<br>
> @@ -1168,7 +1168,7 @@ void DwarfUnit::constructTypeDIE(DIE &Bu<br>
><br>
> bool isPrototyped = true;<br>
> if (Elements.getNumElements() == 2 &&<br>
> - resolve(Elements.getElement(1)).isUnspecifiedParameter())<br>
> + !resolve(Elements.getElement(1)))<br>
<br>
</div>And here (no need for the resolve call).<br>
<div class=""><br>
> isPrototyped = false;<br>
><br>
> constructSubprogramArguments(Buffer, Elements);<br>
><br>
> Modified: llvm/trunk/lib/IR/DIBuilder.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=214189&r1=214188&r2=214189&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=214189&r1=214188&r2=214189&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/IR/DIBuilder.cpp (original)<br>
> +++ llvm/trunk/lib/IR/DIBuilder.cpp Tue Jul 29 13:20:39 2014<br>
> @@ -875,11 +875,8 @@ void DIBuilder::retainType(DIType T) {<br>
><br>
> /// createUnspecifiedParameter - Create unspeicified type descriptor<br>
> /// for the subroutine type.<br>
> -DITrivialType DIBuilder::createUnspecifiedParameter() {<br>
> - Value *Elts[] = {<br>
> - GetTagConstant(VMContext, dwarf::DW_TAG_unspecified_parameters)<br>
> - };<br>
> - return DITrivialType(MDNode::get(VMContext, Elts));<br>
> +DIBasicType DIBuilder::createUnspecifiedParameter() {<br>
> + return DIBasicType(nullptr);<br>
<br>
</div>nullptr is the default argument, so you can omit it (just "return<br>
DIBasicType()").<br>
<div class=""><div class="h5"><br>
> }<br>
><br>
> /// createForwardDecl - Create a temporary forward-declared type that<br>
><br>
> Modified: llvm/trunk/lib/IR/DebugInfo.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=214189&r1=214188&r2=214189&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=214189&r1=214188&r2=214189&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/IR/DebugInfo.cpp (original)<br>
> +++ llvm/trunk/lib/IR/DebugInfo.cpp Tue Jul 29 13:20:39 2014<br>
> @@ -39,7 +39,6 @@ bool DIDescriptor::Verify() const {<br>
> return DbgNode &&<br>
> (DIDerivedType(DbgNode).Verify() ||<br>
> DICompositeType(DbgNode).Verify() || DIBasicType(DbgNode).Verify() ||<br>
> - DITrivialType(DbgNode).Verify() ||<br>
> DIVariable(DbgNode).Verify() || DISubprogram(DbgNode).Verify() ||<br>
> DIGlobalVariable(DbgNode).Verify() || DIFile(DbgNode).Verify() ||<br>
> DICompileUnit(DbgNode).Verify() || DINameSpace(DbgNode).Verify() ||<br>
> @@ -155,10 +154,6 @@ MDNode *DIVariable::getInlinedAt() const<br>
> // Predicates<br>
> //===----------------------------------------------------------------------===//<br>
><br>
> -bool DIDescriptor::isTrivialType() const {<br>
> - return DbgNode && getTag() == dwarf::DW_TAG_unspecified_parameters;<br>
> -}<br>
> -<br>
> bool DIDescriptor::isSubroutineType() const {<br>
> return isCompositeType() && getTag() == dwarf::DW_TAG_subroutine_type;<br>
> }<br>
> @@ -233,8 +228,7 @@ bool DIDescriptor::isVariable() const {<br>
><br>
> /// isType - Return true if the specified tag is legal for DIType.<br>
> bool DIDescriptor::isType() const {<br>
> - return isBasicType() || isCompositeType() || isDerivedType() ||<br>
> - isTrivialType();<br>
> + return isBasicType() || isCompositeType() || isDerivedType();<br>
> }<br>
><br>
> /// isSubprogram - Return true if the specified tag is legal for<br>
> @@ -250,12 +244,6 @@ bool DIDescriptor::isGlobalVariable() co<br>
> getTag() == dwarf::DW_TAG_constant);<br>
> }<br>
><br>
> -/// isUnspecifiedParmeter - Return true if the specified tag is<br>
> -/// DW_TAG_unspecified_parameters.<br>
> -bool DIDescriptor::isUnspecifiedParameter() const {<br>
> - return DbgNode && getTag() == dwarf::DW_TAG_unspecified_parameters;<br>
> -}<br>
> -<br>
> /// isScope - Return true if the specified tag is one of the scope<br>
> /// related tag.<br>
> bool DIDescriptor::isScope() const {<br>
> @@ -459,7 +447,7 @@ bool DIType::Verify() const {<br>
><br>
> // FIXME: Sink this into the various subclass verifies.<br>
> uint16_t Tag = getTag();<br>
> - if (!isBasicType() && !isTrivialType() && Tag != dwarf::DW_TAG_const_type &&<br>
> + if (!isBasicType() && Tag != dwarf::DW_TAG_const_type &&<br>
> Tag != dwarf::DW_TAG_volatile_type && Tag != dwarf::DW_TAG_pointer_type &&<br>
> Tag != dwarf::DW_TAG_ptr_to_member_type &&<br>
> Tag != dwarf::DW_TAG_reference_type &&<br>
> @@ -474,8 +462,6 @@ bool DIType::Verify() const {<br>
> // a CompositeType.<br>
> if (isBasicType())<br>
> return DIBasicType(DbgNode).Verify();<br>
> - else if (isTrivialType())<br>
> - return DITrivialType(DbgNode).Verify();<br>
> else if (isCompositeType())<br>
> return DICompositeType(DbgNode).Verify();<br>
> else if (isDerivedType())<br>
> @@ -489,10 +475,6 @@ bool DIBasicType::Verify() const {<br>
> return isBasicType() && DbgNode->getNumOperands() == 10;<br>
> }<br>
><br>
> -bool DITrivialType::Verify() const {<br>
> - return isTrivialType() && DbgNode->getNumOperands() == 1;<br>
> -}<br>
> -<br>
> /// Verify - Verify that a derived type descriptor is well formed.<br>
> bool DIDerivedType::Verify() const {<br>
> // Make sure DerivedFrom @ field 9 is TypeRef.<br>
> @@ -1297,7 +1279,7 @@ void DIEnumerator::printInternal(raw_ost<br>
> }<br>
><br>
> void DIType::printInternal(raw_ostream &OS) const {<br>
> - if (!DbgNode || isTrivialType())<br>
> + if (!DbgNode)<br>
> return;<br>
><br>
> StringRef Res = getName();<br>
><br>
> Modified: llvm/trunk/test/DebugInfo/varargs.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/varargs.ll?rev=214189&r1=214188&r2=214189&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/varargs.ll?rev=214189&r1=214188&r2=214189&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/DebugInfo/varargs.ll (original)<br>
> +++ llvm/trunk/test/DebugInfo/varargs.ll Tue Jul 29 13:20:39 2014<br>
> @@ -76,16 +76,15 @@ attributes #1 = { nounwind readnone }<br>
> !5 = metadata !{metadata !6}<br>
> !6 = metadata !{i32 786478, metadata !1, metadata !"_ZTS1A", metadata !"a", metadata !"a", metadata !"_ZN1A1aEiz", i32 6, metadata !7, i1 false, i1 false, i32 0, i32 0, null, i32 256, i1 false, null, null, i32 0, metadata !12, i32 6} ; [ DW_TAG_subprogram ] [line 6] [a]<br>
> !7 = metadata !{i32 786453, i32 0, null, metadata !"", i32 0, i64 0, i64 0, i64 0, i32 0, null, metadata !8, i32 0, null, null, null} ; [ DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]<br>
> -!8 = metadata !{null, metadata !9, metadata !10, metadata !11}<br>
> +!8 = metadata !{null, metadata !9, metadata !10, null}<br>
> !9 = metadata !{i32 786447, null, null, metadata !"", i32 0, i64 64, i64 64, i64 0, i32 1088, metadata !"_ZTS1A"} ; [ DW_TAG_pointer_type ] [line 0, size 64, align 64, offset 0] [artificial] [from _ZTS1A]<br>
> !10 = metadata !{i32 786468, null, null, metadata !"int", i32 0, i64 32, i64 32, i64 0, i32 0, i32 5} ; [ DW_TAG_base_type ] [int] [line 0, size 32, align 32, offset 0, enc DW_ATE_signed]<br>
> -!11 = metadata !{i32 786456}<br>
> !12 = metadata !{i32 786468}<br>
> !13 = metadata !{metadata !14}<br>
> !14 = metadata !{i32 786478, metadata !1, metadata !15, metadata !"b", metadata !"b", metadata !"_Z1biz", i32 13, metadata !16, i1 false, i1 true, i32 0, i32 0, null, i32 256, i1 false, void (i32, ...)* @_Z1biz, null, null, metadata !2, i32 13} ; [ DW_TAG_subprogram ] [line 13] [def] [b]<br>
> !15 = metadata !{i32 786473, metadata !1} ; [ DW_TAG_file_type ] [llvm/tools/clang/test/CodeGenCXX/debug-info-varargs.cpp]<br>
> !16 = metadata !{i32 786453, i32 0, null, metadata !"", i32 0, i64 0, i64 0, i64 0, i32 0, null, metadata !17, i32 0, null, null, null} ; [ DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]<br>
> -!17 = metadata !{null, metadata !10, metadata !11}<br>
> +!17 = metadata !{null, metadata !10, null}<br>
> !18 = metadata !{i32 2, metadata !"Dwarf Version", i32 2}<br>
> !19 = metadata !{i32 1, metadata !"Debug Info Version", i32 1}<br>
> !20 = metadata !{metadata !"clang version 3.5 "}<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div></div>