https://github.com/santhoshe447 updated https://github.com/llvm/llvm-project/pull/207359
>From 099493c87a59ee22324fb2d5a96b2dc6b597cd1a Mon Sep 17 00:00:00 2001 From: Santhosh Kumar Ellendula <[email protected]> Date: Fri, 3 Jul 2026 01:24:26 -0700 Subject: [PATCH] [lldb] Improve VLIW(Hexagon) instruction packet disassembly formatting The existing LLDB instruction formatting logic is handles single instructions and does not correctly present VLIW instruction packets, particularly for Hexagon multi-instruction packets. This change refactors the instruction formatting path to support VLIW packet presentation without modifying the existing disassembler or instruction decoding logic. This also helps avoid merge conflicts in downstream repo. Implementation details: Formatting is performed through the virtual method DumpMnemonicOperandsAndComment(), which is defined in Instruction and overridden in InstructionLLVMC. This change moves the common formatting logic from Instruction::Dump() into Instruction::DumpMnemonicOperandsAndComment(), allowing InstructionLLVMC to select the appropriate formatting path based on the instruction type. 1. Non VLIW instructions continue to use the existing formatting logic by delegating to Instruction::DumpMnemonicOperandsAndComment(). 2. VLIW instructions are handled through DumpVLIWMnemonicOperandsAndComment(), which provides generic packet formatting support. This refactoring preserves existing behavior for non VLIW targets while providing an extensible framework for supporting VLIW architectures such as Hexagon. Change-Id: I05020a602b73e27b6c66d666a077bb55796a077b --- lldb/include/lldb/Core/Disassembler.h | 16 ++ lldb/source/Core/Disassembler.cpp | 92 ++++++---- .../Disassembler/LLVMC/DisassemblerLLVMC.cpp | 169 ++++++++++++++++-- 3 files changed, 221 insertions(+), 56 deletions(-) diff --git a/lldb/include/lldb/Core/Disassembler.h b/lldb/include/lldb/Core/Disassembler.h index 4930cee61af10..21e94e9053087 100644 --- a/lldb/include/lldb/Core/Disassembler.h +++ b/lldb/include/lldb/Core/Disassembler.h @@ -51,6 +51,7 @@ class Disassembler; class Module; class StackFrame; class Stream; +class StreamString; class SymbolContext; class SymbolContextList; class Target; @@ -234,6 +235,21 @@ class Instruction { static const char *GetNameForInstructionControlFlowKind( lldb::InstructionControlFlowKind instruction_control_flow_kind); + struct DumpContext { + uint32_t max_opcode_byte_size = 0; + bool show_address = false; + bool show_bytes = false; + bool show_control_flow_kind = false; + const ExecutionContext *exe_ctx = nullptr; + const SymbolContext *sym_ctx = nullptr; + const SymbolContext *prev_sym_ctx = nullptr; + const FormatEntity::Entry *disassembly_addr_format = nullptr; + size_t max_address_text_size = 0; + }; + + virtual void DumpMnemonicOperandsAndComment(StreamString &ss, + const DumpContext &ctx); + /// Get variable annotations for this instruction as structured data. /// Returns an array of dictionaries to be used in SBInstruction class. StructuredData::ArraySP GetVariableAnnotations(); diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index 544f20d7bc897..928968a2de4ea 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -822,74 +822,64 @@ const char *Instruction::GetNameForInstructionControlFlowKind( llvm_unreachable("Fully covered switch above!"); } -void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size, - bool show_address, bool show_bytes, - bool show_control_flow_kind, - const ExecutionContext *exe_ctx, - const SymbolContext *sym_ctx, - const SymbolContext *prev_sym_ctx, - const FormatEntity::Entry *disassembly_addr_format, - size_t max_address_text_size) { +void Instruction::DumpMnemonicOperandsAndComment(StreamString &ss, + const DumpContext &ctx) { size_t opcode_column_width = 7; const size_t operand_column_width = 25; - CalculateMnemonicOperandsAndCommentIfNeeded(exe_ctx); - - StreamString ss; - - if (show_address) { - Debugger::FormatDisassemblerAddress(disassembly_addr_format, sym_ctx, - prev_sym_ctx, exe_ctx, &m_address, ss); - ss.FillLastLineToColumn(max_address_text_size, ' '); + if (ctx.show_address) { + Debugger::FormatDisassemblerAddress(ctx.disassembly_addr_format, + ctx.sym_ctx, ctx.prev_sym_ctx, + ctx.exe_ctx, &m_address, ss); + ss.FillLastLineToColumn(ctx.max_address_text_size, ' '); } - if (show_bytes) { + if (ctx.show_bytes) { if (m_opcode.GetType() == Opcode::eTypeBytes) { - // x86_64 and i386 are the only ones that use bytes right now so pad out - // the byte dump to be able to always show 15 bytes (3 chars each) plus a - // space - if (max_opcode_byte_size > 0) - m_opcode.Dump(&ss, max_opcode_byte_size * 3 + 1); + // x86_64 and i386 are the only ones that use bytes right now so pad + // out the byte dump to be able to always show 15 bytes (3 chars each) + // plus a space + if (ctx.max_opcode_byte_size > 0) + m_opcode.Dump(&ss, ctx.max_opcode_byte_size * 3 + 1); else m_opcode.Dump(&ss, 15 * 3 + 1); } else { - // Else, we have ARM or MIPS which can show up to a uint32_t 0x00000000 - // (10 spaces) plus two for padding... - if (max_opcode_byte_size > 0) - m_opcode.Dump(&ss, max_opcode_byte_size * 3 + 1); + // Else, we have ARM or MIPS which can show up to a uint32_t + // 0x00000000 (10 spaces) plus two for padding... + if (ctx.max_opcode_byte_size > 0) + m_opcode.Dump(&ss, ctx.max_opcode_byte_size * 3 + 1); else m_opcode.Dump(&ss, 12); } } - if (show_control_flow_kind) { + if (ctx.show_control_flow_kind) { lldb::InstructionControlFlowKind instruction_control_flow_kind = - GetControlFlowKind(exe_ctx); + GetControlFlowKind(ctx.exe_ctx); ss.Printf("%-12s", GetNameForInstructionControlFlowKind( instruction_control_flow_kind)); } bool show_color = false; - if (exe_ctx) { - if (TargetSP target_sp = exe_ctx->GetTargetSP()) { + if (ctx.exe_ctx) { + if (TargetSP target_sp = ctx.exe_ctx->GetTargetSP()) show_color = target_sp->GetDebugger().GetUseColor(); - } } + const size_t opcode_pos = ss.GetSizeOfLastLine(); std::string &opcode_name = show_color ? m_markup_opcode_name : m_opcode_name; const std::string &mnemonics = show_color ? m_markup_mnemonics : m_mnemonics; - if (opcode_name.empty()) - opcode_name = "<unknown>"; - // The default opcode size of 7 characters is plenty for most architectures // but some like arm can pull out the occasional vqrshrun.s16. We won't get - // consistent column spacing in these cases, unfortunately. Also note that we - // need to directly use m_opcode_name here (instead of opcode_name) so we + // consistent column spacing in these cases, unfortunately. Also note that + // we need to directly use m_opcode_name here (instead of opcode_name) so we // don't include color codes as characters. - if (m_opcode_name.length() >= opcode_column_width) { + if (m_opcode_name.length() >= opcode_column_width) opcode_column_width = m_opcode_name.length() + 1; - } + + if (opcode_name.empty()) + opcode_name = "<unknown>"; ss.PutCString(opcode_name); ss.FillLastLineToColumn(opcode_pos + opcode_column_width, ' '); @@ -901,6 +891,32 @@ void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size, ss.PutCString(" ; "); ss.PutCString(m_comment); } +} + +void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size, + bool show_address, bool show_bytes, + bool show_control_flow_kind, + const ExecutionContext *exe_ctx, + const SymbolContext *sym_ctx, + const SymbolContext *prev_sym_ctx, + const FormatEntity::Entry *disassembly_addr_format, + size_t max_address_text_size) { + CalculateMnemonicOperandsAndCommentIfNeeded(exe_ctx); + + StreamString ss; + + DumpContext ctx; + ctx.max_opcode_byte_size = max_opcode_byte_size; + ctx.show_address = show_address; + ctx.show_bytes = show_bytes; + ctx.show_control_flow_kind = show_control_flow_kind; + ctx.exe_ctx = exe_ctx; + ctx.sym_ctx = sym_ctx; + ctx.prev_sym_ctx = prev_sym_ctx; + ctx.disassembly_addr_format = disassembly_addr_format; + ctx.max_address_text_size = max_address_text_size; + + DumpMnemonicOperandsAndComment(ss, ctx); s->PutCString(ss.GetString()); } diff --git a/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp b/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp index 6129967ce65bc..20103bfa9ba2c 100644 --- a/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp +++ b/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp @@ -45,6 +45,7 @@ #include "lldb/Utility/Log.h" #include "lldb/Utility/RegularExpression.h" #include "lldb/Utility/Stream.h" +#include "lldb/Utility/StreamString.h" #include <algorithm> #include <optional> @@ -423,7 +424,10 @@ class InstructionLLVMC : public lldb_private::Instruction { AddressClass addr_class) : Instruction(address, addr_class), m_disasm_wp(std::static_pointer_cast<DisassemblerLLVMC>( - disasm.shared_from_this())) {} + disasm.shared_from_this())), + // Add new VLIW targets here. + m_is_vliw(disasm.GetArchitecture().GetMachine() == + llvm::Triple::hexagon) {} ~InstructionLLVMC() override = default; @@ -594,10 +598,13 @@ class InstructionLLVMC : public lldb_private::Instruction { bool use_hex_immediates = true; Disassembler::HexImmediateStyle hex_style = Disassembler::eHexStyleC; + bool is_hexagon = false; if (exe_ctx) { Target *target = exe_ctx->GetTargetPtr(); if (target) { + is_hexagon = (target->GetArchitecture().GetMachine() == + llvm::Triple::hexagon); use_hex_immediates = target->GetUseHexImmediates(); hex_style = target->GetHexImmediateStyle(); @@ -681,19 +688,29 @@ class InstructionLLVMC : public lldb_private::Instruction { m_mnemonics = std::string(mnemonic_strm.GetString()); return; } + if (is_hexagon) { + // Hexagon packets are multi-line strings separated by '\n'. + // The complete string (including '\v' separators) is stored in + // m_opcode_name and used by Dump() to display the packet. + // Other VLIW architectures should make sure this is true for + // their disassembly as well. + m_opcode_name = out_string; + m_markup_opcode_name = markup_out_string; + } else { - static RegularExpression s_regex( - llvm::StringRef("[ \t]*([^ ^\t]+)[ \t]*([^ ^\t].*)?")); + static RegularExpression s_regex( + llvm::StringRef("[ \t]*([^ ^\t]+)[ \t]*([^ ^\t].*)?")); - llvm::SmallVector<llvm::StringRef, 4> matches; - if (s_regex.Execute(out_string, &matches)) { - m_opcode_name = matches[1].str(); - m_mnemonics = matches[2].str(); - } - matches.clear(); - if (s_regex.Execute(markup_out_string, &matches)) { - m_markup_opcode_name = matches[1].str(); - m_markup_mnemonics = matches[2].str(); + llvm::SmallVector<llvm::StringRef, 4> matches; + if (s_regex.Execute(out_string, &matches)) { + m_opcode_name = matches[1].str(); + m_mnemonics = matches[2].str(); + } + matches.clear(); + if (s_regex.Execute(markup_out_string, &matches)) { + m_markup_opcode_name = matches[1].str(); + m_markup_mnemonics = matches[2].str(); + } } } } @@ -1177,9 +1194,93 @@ class InstructionLLVMC : public lldb_private::Instruction { return m_is_call; } + void DumpMnemonicOperandsAndComment(StreamString &ss, + const DumpContext &ctx) override { + if (m_is_vliw) + DumpVLIWMnemonicOperandsAndComment(ss, ctx); + else + Instruction::DumpMnemonicOperandsAndComment(ss, ctx); + } + + void DumpVLIWMnemonicOperandsAndComment(StreamString &ss, + const DumpContext &ctx) { + // Set bundle start/end delimiters based on the VLIW architecture. + // Add cases for new VLIW targets here. + const char *bundle_start = nullptr; + const char *bundle_end = nullptr; + DisassemblerSP disasm_sp = m_disasm_wp.lock(); + const bool is_hexagon = + disasm_sp && + disasm_sp->GetArchitecture().GetMachine() == llvm::Triple::hexagon; + if (is_hexagon) { + bundle_start = "{ "; + bundle_end = " }"; + } + + llvm::StringRef packetStringRef(m_opcode_name); + std::pair<llvm::StringRef, llvm::StringRef> splitPacket = + packetStringRef.rsplit('\n'); + llvm::StringRef postScript = splitPacket.second; + splitPacket = splitPacket.first.split('\n'); + const char *preamble = bundle_start; + const char *postamble = bundle_end; + bool printPostamble = true; + bool done = false; + do { + if (ctx.show_address) { + Debugger::FormatDisassemblerAddress(ctx.disassembly_addr_format, + ctx.sym_ctx, ctx.prev_sym_ctx, + ctx.exe_ctx, &m_address, ss); + const uint32_t min_inst_byte_size = + disasm_sp ? disasm_sp->GetArchitecture().GetMinimumOpcodeByteSize() + : 4; + m_address.Slide(min_inst_byte_size); + ss.FillLastLineToColumn(ctx.max_address_text_size, ' '); + } + if (ctx.show_bytes) + m_opcode.Dump(&ss, 12); + if (splitPacket.first.empty()) { + ss.PutCString("<unknown>"); + printPostamble = false; + done = true; + } else { + ss.PutCString(preamble); + preamble = " "; + // Hexagon duplex instructions are two sub-instructions packed + // into one instruction slot, separated by '\v'. + if (is_hexagon && + splitPacket.first.find('\v') != llvm::StringRef::npos) { + std::pair<llvm::StringRef, llvm::StringRef> duplex = + splitPacket.first.split('\v'); + ss.PutCString(duplex.first.str().c_str()); + ss.PutChar('\n'); + ss.FillLastLineToColumn(ctx.max_address_text_size + 4, ' '); + ss.PutCString(duplex.second.str().c_str()); + } else + ss.PutCString(splitPacket.first.str().c_str()); + splitPacket = splitPacket.second.split('\n'); + done = splitPacket.first.empty(); + if (!done) + ss.PutChar('\n'); + } + } while (!done); + if (printPostamble) + ss.PutCString(postamble); + if (!m_comment.empty()) { + ss.PutCString(" ; "); + ss.PutCString(m_comment.c_str()); + } + if (!postScript.empty()) { + ss.PutChar('\n'); + ss.FillLastLineToColumn(ctx.max_address_text_size, ' '); + ss.PutCString(postScript.str().c_str()); + } + } + protected: std::weak_ptr<DisassemblerLLVMC> m_disasm_wp; + bool m_is_vliw = false; bool m_is_valid = false; bool m_using_file_addr = false; bool m_has_visited_instruction = false; @@ -1209,7 +1310,7 @@ class InstructionLLVMC : public lldb_private::Instruction { if (!m_opcode.GetData(data)) return; - bool is_alternate_isa; + bool is_alternate_isa = false; lldb::addr_t pc = m_address.GetFileAddress(); DisassemblerLLVMC::MCDisasmInstance *mc_disasm_ptr = GetDisasmToUse(is_alternate_isa, disasm); @@ -1229,6 +1330,27 @@ class InstructionLLVMC : public lldb_private::Instruction { m_is_load = mc_disasm_ptr->IsLoad(inst); m_is_authenticated = mc_disasm_ptr->IsAuthenticated(inst); m_is_barrier = mc_disasm_ptr->IsBarrier(inst); + + bool does_branch = m_does_branch; + bool has_delay_slot = m_has_delay_slot; + bool is_call = m_is_call; + + // The first operand of a Hexagon bundle is an immediate with flags. + // Skip it. + // Iterate over the rest of the operands, calling CanBranch(). + for (unsigned i = 1; i < inst.getNumOperands(); i++) { + llvm::MCOperand op = inst.getOperand(i); + if (!op.isInst()) + continue; + const llvm::MCInst *cmi = op.getInst(); + llvm::MCInst mi = *cmi; + does_branch = mc_disasm_ptr->CanBranch(mi); + has_delay_slot = mc_disasm_ptr->HasDelaySlot(inst); + is_call = mc_disasm_ptr->IsCall(inst); + } + m_does_branch = does_branch; + m_has_delay_slot = has_delay_slot; + m_is_call = is_call; } private: @@ -1360,11 +1482,22 @@ bool DisassemblerLLVMC::MCDisasmInstance::GetMCInst(const uint8_t *opcode_data, llvm::ArrayRef<uint8_t> data(opcode_data, opcode_data_len); llvm::MCDisassembler::DecodeStatus status; - status = m_disasm_up->getInstruction(mc_inst, size, data, pc, llvm::nulls()); - if (status == llvm::MCDisassembler::Success) - return true; - else - return false; + const bool is_hexagon = + m_context_up && + m_context_up->getTargetTriple().getArch() == llvm::Triple::hexagon; + + if (is_hexagon) { + status = m_disasm_up->getInstructionBundle(mc_inst, size, data, pc, + llvm::nulls()); + if (status == llvm::MCDisassembler::Fail) + status = + m_disasm_up->getInstruction(mc_inst, size, data, pc, llvm::nulls()); + } else { + status = + m_disasm_up->getInstruction(mc_inst, size, data, pc, llvm::nulls()); + } + + return status == llvm::MCDisassembler::Success; } void DisassemblerLLVMC::MCDisasmInstance::PrintMCInst( _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
