[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

dblaikie added a comment. This wasn't well committed, unfortunately. It seems it was committed in two parts: IR support and clang functionality: https://github.com/llvm/llvm-project/commit/7a42babeb83e3927e89e72a0e7e45be9d41b6c23 (a recommit of

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

vsk added a comment. @awpandey @SouraVX this is still breaking the stage2 clang self-host with -gdwarf-5, see https://bugs.llvm.org/show_bug.cgi?id=45166. Do you have time to take a look? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73462/new/

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

This revision was automatically updated to reflect the committed changes. Closed by commit rG7a42babeb83e: Reland [DebugInfo][clang][DWARF5]: Added support for debuginfo generation for… (authored by awpandey, committed by SouraVX). Changed prior to commit:

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

hans added a comment. I also just noticed that this broke the Chromium build, see the attached reproducer at https://bugs.chromium.org/p/chromium/issues/detail?id=1057559#c1 clang++: /work/llvm.monorepo/clang/lib/AST/ExprConstant.cpp:14013: llvm::APSInt clang::Expr::EvaluateKnownConstInt(const

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

hans added a comment. In D73462#1900364 , @omjavaid wrote: > This change breaks lldb buildbots here: > > http://lab.llvm.org:8011/builders/lldb-aarch64-ubuntu/builds/2123 > http://lab.llvm.org:8011/builders/lldb-x86_64-debian/builds/5911 > > Please

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

omjavaid reopened this revision. omjavaid added a comment. This revision is now accepted and ready to land. This change breaks lldb buildbots here: http://lab.llvm.org:8011/builders/lldb-aarch64-ubuntu/builds/2123 http://lab.llvm.org:8011/builders/lldb-x86_64-debian/builds/5911 Please revisit

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

This revision was automatically updated to reflect the committed changes. Closed by commit rGc2b437d53d40: [DebugInfo][clang][DWARF5]: Added support for debuginfo generation for… (authored by awpandey, committed by SouraVX). Changed prior to commit:

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

awpandey updated this revision to Diff 247546. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73462/new/ https://reviews.llvm.org/D73462 Files: clang/lib/CodeGen/CGDebugInfo.cpp clang/test/CodeGenCXX/debug-info-template-parameter.cpp llvm/include/llvm/IR/DIBuilder.h

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

aprantl added a comment. LGTM Comment at: llvm/unittests/IR/MetadataTest.cpp:2106 + auto *N = + DITemplateValueParameter::get(Context, Tag, Name, Type, false, Value); EXPECT_EQ(Tag, N->getTag()); For consistency, let's assign "false" to `bool

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

awpandey updated this revision to Diff 245875. awpandey marked 7 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73462/new/ https://reviews.llvm.org/D73462 Files: clang/lib/CodeGen/CGDebugInfo.cpp clang/test/CodeGenCXX/debug-info-template-parameter.cpp

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. LGTM with outstanding inline comments addressed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73462/new/ https://reviews.llvm.org/D73462

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

aprantl added inline comments. Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:2187 + DEFINE_MDNODE_GET(DITemplateTypeParameter, +(MDString * Name, Metadata *Type, bool IsDefault), +(Name, Type, IsDefault))

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

awpandey added a comment. Hi @aprantl, I have included all of your suggestions. Can I merge this? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73462/new/ https://reviews.llvm.org/D73462 ___ cfe-commits mailing list

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

awpandey updated this revision to Diff 244571. awpandey marked 2 inline comments as done. awpandey added a comment. Hi @aprantl, I have added a test that two DITemplateTypeParameter only differ in the isDefault parameter are not equal. CHANGES SINCE LAST ACTION

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

aprantl added a comment. The upgrade test look good, I found a missing test in the unit tests though. Comment at: llvm/unittests/IR/MetadataTest.cpp:2089 + EXPECT_NE(N, DITemplateTypeParameter::get(Context, Name, +

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

awpandey added a comment. Hi @aprantl , can you please inform me what extra changes should I make in this patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73462/new/ https://reviews.llvm.org/D73462 ___ cfe-commits mailing list

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

awpandey added a comment. Thanks @dblaikie for all of your suggestions. I will commit this in 3 patches after getting the green signal from @aprantl. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73462/new/ https://reviews.llvm.org/D73462

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

dblaikie added a comment. @aprantl - functionality looks fine to me, could you check/sign off on the bitcode backwards compatibility stuff? (& ultimately this should probably be committed in 3 patches - one for the IR support, then one for Clang to generate that IR, and another for LLVM to use

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

awpandey updated this revision to Diff 242514. awpandey marked 3 inline comments as done. awpandey added a comment. @dblaikie, I have updated the test cases. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73462/new/ https://reviews.llvm.org/D73462 Files:

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

dblaikie added inline comments. Comment at: clang/test/CodeGenCXX/debug-info-template-parameter.cpp:30-32 + foo f1; + foo f2; + foo<> f3; What are these three (6 if you include the fact that each foo has one default template parameter still) testing? Could

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

awpandey updated this revision to Diff 242250. awpandey added a comment. Hi @dblaikie, I removed the condition and put false as the third argument of PrintBool. I have also removed the updated bitcode file. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73462/new/

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

dblaikie added a comment. Might be easier in smaller patches - Bitcode & IR support, then LLVM/codegen emission and Clang/IR gen. > I have to update dityperefs-3.8.ll and dityperefs-3.8.ll.bc because they are > no longer compatible with this patch. The bitcode files in test/Bitcode generally

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

awpandey updated this revision to Diff 241984. awpandey marked 2 inline comments as done. awpandey added a comment. Thanks, @aprantl, and @dblaikie. I have added two new test cases as per your suggestions. I have to update // dityperefs-3.8.ll// and //dityperefs-3.8.ll.bc// because they are no

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

dblaikie added inline comments. Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:1808-1809 Record.push_back(VE.getMetadataOrNullID(N->getType())); + if (M.getDwarfVersion() >= 5) +Record.push_back(N->getDefault());

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

aprantl added a comment. In D73462#1851129 , @awpandey wrote: > @aprantl Thanks for your valuable feedback. > > I have a doubt. You need a .ll file its corresponding llvm bytecode generated > by current llvm (without my patch). Doest this ll file

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

awpandey marked an inline comment as not done. awpandey added a comment. @aprantl Thanks for your valuable feedback. I have a doubt. You need a .ll file its corresponding llvm bytecode generated by current llvm (without my patch). Doest this ll file includes any checks? In the round trip test

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

aprantl added a comment. In addition to the roundtrip test, can you please also add a .bc file test containing a DITemplateTypeParameter / DITemplateValueParameter produced by today's llvm to test the bitcode upgrade code added to MetadataLoader? You'll find examples if you search for .bc

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

awpandey marked an inline comment as done and an inline comment as not done. awpandey added inline comments. Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:1808-1809 Record.push_back(VE.getMetadataOrNullID(N->getType())); + if (M.getDwarfVersion() >= 5) +

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

awpandey updated this revision to Diff 241391. awpandey marked 7 inline comments as done. awpandey added a comment. Hi @dblaikie, I have incorporated your suggestions (renaming variables, test case modification). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73462/new/

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

dblaikie added inline comments. Comment at: llvm/lib/AsmParser/LLParser.cpp:4858 /// ::= !DITemplateValueParameter(tag: DW_TAG_template_value_parameter, -/// name: "V", type: !1, value: i32 7) +/// name: "V",

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

dblaikie added inline comments. Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:2134 protected: + bool isDefault; + Rename this (& the ctor/other parameters) to "IsDefault" to conform to LLVM's naming conventions (

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

awpandey updated this revision to Diff 241141. awpandey added a comment. Thanks @dblaikie for figuring out the missing feature. I have added support for the non-type template parameter too. Please let me know if further modifications are required. I will happily address them. CHANGES SINCE

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

dblaikie added inline comments. Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:2155 friend class MDNode; + bool isDefault; What about default arguments for non-type template parameters? ("template" etc... ) Repository: rG LLVM Github Monorepo

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

aprantl added a comment. Herald added a subscriber: ormris. Can you add a bitcode roundtrip test to test/Assembler? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73462/new/ https://reviews.llvm.org/D73462

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

awpandey created this revision. awpandey added reviewers: probinson, aprantl, dblaikie. Herald added subscribers: llvm-commits, cfe-commits, hiraditya. Herald added projects: clang, LLVM. This patch provides support for //DW_AT_default_value// based on template parameter is default parameter or