[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

This revision was automatically updated to reflect the committed changes. Closed by commit rG581ba35291a6: [RISCV] ELF attribute section for RISC-V. (authored by HsiangKai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74023/new/

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

jhenderson added a comment. In D74023#1949261 , @HsiangKai wrote: > In D74023#1948388 , @MaskRay wrote: > > > The code generally looks good. For unittests, I think we can either make > > llvm-readobj -A canonical

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

HsiangKai updated this revision to Diff 253553. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74023/new/ https://reviews.llvm.org/D74023 Files: lld/ELF/InputFiles.cpp lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

MaskRay added a comment. The code generally looks good. For unittests, I think we can either make llvm-readobj -A canonical or the unittests canonical. If we decide to place tests on one place, we should delete most tests on the other side. My current preference is that we use more of

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

HsiangKai added a comment. In D74023#1948388 , @MaskRay wrote: > The code generally looks good. For unittests, I think we can either make > llvm-readobj -A canonical or the unittests canonical. If we decide to place > tests on one place, we should

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

HsiangKai added a comment. @MaskRay, do you have any other comments about this patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74023/new/ https://reviews.llvm.org/D74023 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

MaskRay added inline comments. Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp:102 + +Streamer.emitIntValue(ELFAttrs::Format_Version, 1); + } `emitInt8` Comment at:

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

HsiangKai marked an inline comment as done. HsiangKai added inline comments. Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp:93 + StringRef String) { + OS << "\t.attribute\t" << Attribute << ", \"" << String

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

jhenderson accepted this revision. jhenderson added a comment. This revision is now accepted and ready to land. I haven't really reviewed the funcional parts of this change in the attribute parser stuff, but everything else LGTM. Please wait for somebody else to review the attribute parser

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

HsiangKai added a comment. I create a patch, D76819 , to apply clang-format to the ELF header file and ARM build attributes files. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74023/new/

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

HsiangKai marked an inline comment as done. HsiangKai added inline comments. Comment at: llvm/lib/Support/ARMBuildAttrs.cpp:66-68 +const TagNameMap llvm::ARMBuildAttrs::ARMAttributeTags(tagData, + sizeof(tagData) / +

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

HsiangKai updated this revision to Diff 252801. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74023/new/ https://reviews.llvm.org/D74023 Files: lld/ELF/InputFiles.cpp lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

HsiangKai added a comment. Harbormaster result: Unit tests pass. 64040 tests passed, 0 failed and 650 were skipped. clang-tidy fail. clang-tidy found 1 errors and 18 warnings. 0 of them are added as review comments why?. clang-format fail. Please format your changes with clang-format by

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

MaskRay added a comment. I remember the patch caused many failures yesterday. The new diff is good. Comment at: llvm/unittests/Support/ELFAttributeParserTest.cpp:16 + +TagNameMap emptyTagNameMap; + `static const` Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

HsiangKai updated this revision to Diff 252729. HsiangKai added a comment. Rebase on D76819 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74023/new/ https://reviews.llvm.org/D74023 Files:

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

HsiangKai updated this revision to Diff 252710. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74023/new/ https://reviews.llvm.org/D74023 Files: lld/ELF/InputFiles.cpp lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

HsiangKai updated this revision to Diff 252591. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74023/new/ https://reviews.llvm.org/D74023 Files: lld/ELF/InputFiles.cpp lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

HsiangKai updated this revision to Diff 252562. HsiangKai added a comment. Fix Harbormaster test failures. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74023/new/ https://reviews.llvm.org/D74023 Files: lld/ELF/InputFiles.cpp

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

HsiangKai updated this revision to Diff 252505. HsiangKai added a comment. Rebase on master. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74023/new/ https://reviews.llvm.org/D74023 Files: lld/ELF/InputFiles.cpp

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

HsiangKai added a comment. In D74023#1938583 , @jhenderson wrote: > In D74023#1937220 , @HsiangKai wrote: > > > In D74023#1933427 , @jhenderson > > wrote: > > > > >

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

HsiangKai updated this revision to Diff 252300. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74023/new/ https://reviews.llvm.org/D74023 Files: lld/ELF/InputFiles.cpp lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

HsiangKai marked 3 inline comments as done. HsiangKai added inline comments. Comment at: llvm/include/llvm/Support/ARMBuildAttributes.h:37 File = 1, CPU_raw_name = 4, CPU_name = 5, jhenderson wrote: >

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

jhenderson added a comment. In D74023#1937220 , @HsiangKai wrote: > In D74023#1933427 , @jhenderson > wrote: > > > @HsiangKai, have you noticed that there are some test failures according to > > the harbourmaster

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

HsiangKai added a comment. In D74023#1933427 , @jhenderson wrote: > @HsiangKai, have you noticed that there are some test failures according to > the harbourmaster bot? They look like they could be related to this somehow. @jhenderson, yes, I found

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

jhenderson added a comment. @HsiangKai, have you noticed that there are some test failures according to the harbourmaster bot? They look like they could be related to this somehow. Comment at: llvm/unittests/Support/ELFAttributeParserTest.cpp:21 + Error handler(uint64_t tag,

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

HsiangKai updated this revision to Diff 251375. HsiangKai added a comment. Replace std::find_if with llvm::find_if. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74023/new/ https://reviews.llvm.org/D74023 Files: lld/ELF/InputFiles.cpp

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

HsiangKai marked 2 inline comments as done. HsiangKai added inline comments. Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:1727 + + if (Tag % 2) +IsIntegerValue = false; jhenderson wrote: > I don't understand why this line changed, but more

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

HsiangKai updated this revision to Diff 251244. HsiangKai added a comment. Address @jhenderson's comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74023/new/ https://reviews.llvm.org/D74023 Files: lld/ELF/InputFiles.cpp

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

HsiangKai updated this revision to Diff 251287. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74023/new/ https://reviews.llvm.org/D74023 Files: lld/ELF/InputFiles.cpp lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

jhenderson added inline comments. Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:1727 + + if (Tag % 2) +IsIntegerValue = false; I don't understand why this line changed, but more importantly, the `2` looks like a magic number and it is

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

HsiangKai marked 2 inline comments as done. HsiangKai added inline comments. Comment at: llvm/include/llvm/Support/ELFAttributes.h:32 + bool HasTagPrefix = true); +Optional attrTypeFromString(StringRef Tag, TagNameMap Map); + jhenderson

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

HsiangKai updated this revision to Diff 249854. HsiangKai added a comment. Fix unit test errors. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74023/new/ https://reviews.llvm.org/D74023 Files: lld/ELF/InputFiles.cpp

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

HsiangKai updated this revision to Diff 249845. HsiangKai added a comment. - Specify underlying type for AttrType. - Update unit tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74023/new/ https://reviews.llvm.org/D74023 Files:

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

jhenderson added inline comments. Comment at: llvm/include/llvm/Support/ELFAttributes.h:32 + bool HasTagPrefix = true); +Optional attrTypeFromString(StringRef Tag, TagNameMap Map); + HsiangKai wrote: > jhenderson wrote: > > Can this be

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

jhenderson added inline comments. Comment at: llvm/unittests/Support/ELFAttributeParser.cpp:5 +#include "gtest/gtest.h" +#include + jhenderson wrote: > I think it's normal to put a blank line between standard library headers and > other includes. Ignore this.

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

HsiangKai updated this revision to Diff 249024. HsiangKai added a comment. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Move llvm-readobj changes and tests to D75833 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION