[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

2022-07-08 Thread Denis via Phabricator via lldb-commits
treapster abandoned this revision. treapster added a comment. Abandoning because @MaskRay already implemented it in D128029 and D128030 . Thanks for your time, @MaskRay! Repository: rG LLVM Github Monorepo CHANGES SINCE

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

2022-06-28 Thread Denis via Phabricator via lldb-commits
treapster added a comment. In D127741#3583559 , @DavidSpickett wrote: > > I agree that objdump should have some kind of "max" setting, even a default. > However there will still need to be a way to test what I referred to. As you said, `

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

2022-06-28 Thread Stephen Hines via Phabricator via lldb-commits
srhines added a comment. We had some internal Google folks hit this exact issue recently. I don't think that the same "default" CPU should be used for debugging tools like `llvm-objdump`, and that is the crux of the matter here. Perhaps updating the test to specify "generic" as the CPU when

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

2022-06-28 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. > Why not use objdump? If we only check that instructions get disassembled, it > will work fine. I just remember some tests using llvm-mc to disassemble and some using objdump. Istr that using llvm-mc for assemble and disassemble usually meant having two test

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

2022-06-28 Thread Denis via Phabricator via lldb-commits
treapster added a comment. For now i'd like to hear more about possible clashing instruction encodings from @labrinea, and if it's not a concern, i think we can fix the tests if we remove `CHECK-UNKNOWN:` and only call objdump with `CHECK-INST`. What do you think? Repository: rG LLVM

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

2022-06-28 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. Now I think about it, tests could do something like "-mattr=-all" to remove it, right? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127741/new/ https://reviews.llvm.org/D127741

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

2022-06-28 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. I like the idea of `all` for llvm-objdump, but it should be a separate change, with `-mattr=-all` tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127741/new/ https://reviews.llvm.org/D127741

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

2022-06-28 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added subscribers: labrinea, DavidSpickett. DavidSpickett added a reviewer: DavidSpickett. DavidSpickett added a comment. Herald added a subscriber: Michael137. > conflicting instructions with the same encoding in different extensions, but > it doesn't seem possible within one

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

2022-06-28 Thread Denis via Phabricator via lldb-commits
treapster added a comment. Well, it broke a lot of tests that very explicitly checking that instructions are not disassembled without explicitly specified extensions, but i don't quite get the point of such tests. If new instructions were added by default in lldb, why can't we do the same with

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

2022-06-28 Thread Denis via Phabricator via lldb-commits
treapster created this revision. treapster added a reviewer: rafaelauler. Herald added subscribers: jsji, ayermolo, pengfei, rupprecht, hiraditya, kristof.beyls. Herald added a reviewer: jhenderson. Herald added a reviewer: MaskRay. Herald added a reviewer: rafauler. Herald added a reviewer: