wallace added inline comments.
================
Comment at: lldb/include/lldb/Core/Disassembler.h:82-86
+ lldb::InstructionControlFlowType
+ instruction_decode(uint8_t opcode, uint8_t modrm, uint8_t map);
+
+ lldb::InstructionControlFlowType
+ GetControlFlowInstructionKind(const ExecutionContext *exe_ctx);
----------------
as this code is new, please write documentation and function names should be in
PascalCase. Try to have a more descriptive name as well, because
`instruction_decode` is very vague.
================
Comment at: lldb/include/lldb/Core/Disassembler.h:152
virtual void Dump(Stream *s, uint32_t max_opcode_byte_size, bool
show_address,
- bool show_bytes, const ExecutionContext *exe_ctx,
+ bool show_bytes, bool show_kind,
+ const ExecutionContext *exe_ctx,
----------------
let's try to be more explicit with the naming
================
Comment at: lldb/include/lldb/Core/Disassembler.h:226-233
+ enum {
+ PTI_MAP_0,
+ PTI_MAP_1,
+ PTI_MAP_2,
+ PTI_MAP_3,
+ PTI_MAP_AMD3DNOW,
+ PTI_MAP_INVALID
----------------
add documentation. Also, if this mapping is intel-specific, I recommend moving
this to the cpp file so that it's not exposed publicly. The Disassembler is
architecture agnostic, so its API should try to remain that way
================
Comment at: lldb/include/lldb/Core/Disassembler.h:341
- void Dump(Stream *s, bool show_address, bool show_bytes,
+ void Dump(Stream *s, bool show_address, bool show_bytes, bool show_kind,
const ExecutionContext *exe_ctx);
----------------
name
================
Comment at: lldb/include/lldb/Core/Disassembler.h:398
+ (1u << 3), // Mark the disassembly line the contains the PC
+ eOptionShowKind = (1u << 4),
};
----------------
================
Comment at: lldb/include/lldb/Target/TraceCursor.h:35
/// point at them. The consumer should invoke \a TraceCursor::GetError() to
/// check if the cursor is pointing to either a valid instruction or an error.
///
----------------
this file has changed, could you rebase from upstream/main.
================
Comment at: lldb/include/lldb/Target/TraceCursor.h:230-234
/// \return
- /// The \a lldb::TraceInstructionControlFlowType categories the
+ /// The \a lldb::InstructionControlFlowType categories the
/// instruction the cursor is pointing at falls into. If the cursor
points
/// to an error in the trace, return \b 0.
+ virtual lldb::InstructionControlFlowType GetInstructionControlFlowType() = 0;
----------------
when you rebase, could you delete any references to this method and its
implementation as well? We won't use it anymore
================
Comment at: lldb/include/lldb/Target/TraceInstructionDumper.h:38
+ /// For each instruction, print the instruction type.
+ bool show_kind = false;
/// Optional custom id to start traversing from.
----------------
name
================
Comment at: lldb/include/lldb/lldb-enumerations.h:973
/// A single instruction can match one or more of these categories.
-FLAGS_ENUM(TraceInstructionControlFlowType){
- /// Any instruction.
- eTraceInstructionControlFlowTypeInstruction = (1u << 1),
- /// A conditional or unconditional branch/jump.
- eTraceInstructionControlFlowTypeBranch = (1u << 2),
- /// A conditional or unconditional branch/jump that changed
- /// the control flow of the program.
- eTraceInstructionControlFlowTypeTakenBranch = (1u << 3),
- /// A call to a function.
- eTraceInstructionControlFlowTypeCall = (1u << 4),
- /// A return from a function.
- eTraceInstructionControlFlowTypeReturn = (1u << 5)};
-
-LLDB_MARK_AS_BITMASK_ENUM(TraceInstructionControlFlowType)
+FLAGS_ENUM(InstructionControlFlowType){
+ /// The instruction could not be classified.
----------------
this doesn't need to be a FLAGS_ENUM anymore (i.e. bitmask), you can use a
simple enum instead, like the `ExpressionEvaluationPhase` above
================
Comment at: lldb/source/API/SBInstruction.cpp:244
FormatEntity::Parse("${addr}: ", format);
- inst_sp->Dump(&s.ref(), 0, true, false, nullptr, &sc, nullptr, &format, 0);
+ inst_sp->Dump(&s.ref(), 0, true, false, false, nullptr, &sc, nullptr,
+ &format, 0);
----------------
include this kind of comment wherever you have added this parameter
================
Comment at: lldb/source/Commands/CommandObjectDisassemble.h:49
bool show_bytes;
+ bool show_kind;
uint32_t num_lines_context = 0;
----------------
ame
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128477/new/
https://reviews.llvm.org/D128477
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
