llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Dave Lee (kastiglione) <details> <summary>Changes</summary> Adopt a single representation for integers in the formatter bytecode interpreter. This simplifies code generation, where the (now deprecated) distinction of `uint64_t` and `int64_t` types could make conditionals and other operations more complicated, by having to have code paths for signed and unsigned. Depends on https://github.com/llvm/llvm-project/pull/218801 Assistec-by: claude --- Patch is 27.25 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/220665.diff 7 Files Affected: - (modified) lldb/docs/resources/formatterbytecode.md (+15-10) - (modified) lldb/include/lldb/DataFormatters/FormatterBytecode.def (+5-4) - (modified) lldb/include/lldb/DataFormatters/FormatterBytecode.h (+14-2) - (modified) lldb/source/DataFormatters/FormatterBytecode.cpp (+139-19) - (modified) lldb/source/DataFormatters/TypeSummary.cpp (+2) - (modified) lldb/source/DataFormatters/TypeSynthetic.cpp (+19) - (modified) lldb/unittests/DataFormatter/FormatterBytecodeTest.cpp (+125) ``````````diff diff --git a/lldb/docs/resources/formatterbytecode.md b/lldb/docs/resources/formatterbytecode.md index 8cfef645b3352..dbd83b7471d70 100644 --- a/lldb/docs/resources/formatterbytecode.md +++ b/lldb/docs/resources/formatterbytecode.md @@ -27,8 +27,9 @@ The virtual machine has two stacks, a data and a control stack. The control stac All objects on the data stack must have one of the following data types. These data types are "host" data types, in LLDB parlance. - *String* (UTF-8) -- *Int* (64 bit) -- *UInt* (64 bit) +- *Integer* (arbitrary precision, always signed) +- *Int* (64 bit) (deprecated: use *Integer*) +- *UInt* (64 bit) (deprecated: use *Integer*) - *Object* (Basically an `SBValue`) - *Type* (Basically an `SBType`) - *Selector* (One of the predefine functions) @@ -56,7 +57,7 @@ These instructions manipulate the data stack directly. ### Control flow -These manipulate the control stack and program counter. Both `if` and `ifelse` expect a `UInt` at the top of the data stack to represent the condition. +These manipulate the control stack and program counter. Both `if` and `ifelse` expect an `Integer` or `UInt` at the top of the data stack to represent the condition. ```{eval-rst} ======== ============ ============================================================ @@ -64,9 +65,9 @@ These manipulate the control stack and program counter. Both `if` and `ifelse` e -------- ------------ ------------------------------------------------------------ 0x10 ``{`` push a code block address onto the control stack -- ``}`` (technically not an opcode) syntax for end of code block - 0x11 ``if`` ``(UInt -> )`` pop a block from the control stack, + 0x11 ``if`` ``(Integer|UInt -> )`` pop a block from the control stack, if the top of the data stack is nonzero, execute it - 0x12 ``ifelse`` ``(UInt -> )`` pop two blocks from the control stack, if + 0x12 ``ifelse`` ``(Integer|UInt -> )`` pop two blocks from the control stack, if the top of the data stack is nonzero, execute the first, otherwise the second. 0x13 ``return`` pop the entire control stack and return @@ -79,11 +80,12 @@ These manipulate the control stack and program counter. Both `if` and `ifelse` e ======== ============= ============================================================ Opcode Mnemonic Description -------- ------------- ------------------------------------------------------------ - 0x20 ``123u`` ``( -> UInt)`` push an unsigned 64-bit host integer - 0x21 ``123`` ``( -> Int)`` push a signed 64-bit host integer + 0x20 ``123u`` ``( -> UInt)`` push an unsigned 64-bit host integer (deprecated: use ``lit_integer``) + 0x21 ``123`` ``( -> Int)`` push a signed 64-bit host integer (deprecated: use ``lit_integer``) 0x22 ``"abc"`` ``( -> String)`` push a UTF-8 host string 0x23 ``@strlen`` ``( -> Selector)`` push one of the predefined function selectors. See ``call``. + 0x24 ``123`` ``( -> Integer)`` push an arbitrary precision signed integer ======== ============= ============================================================ ``` @@ -93,15 +95,17 @@ These manipulate the control stack and program counter. Both `if` and `ifelse` e ======== ============= ================================================================ Opcode Mnemonic Description -------- ------------- ---------------------------------------------------------------- - 0x2a ``as_int`` ``( UInt -> Int)`` reinterpret a UInt as an Int - 0x2b ``as_uint`` ``( Int -> UInt)`` reinterpret an Int as a UInt + 0x2a ``as_int`` ``( UInt -> Int)`` reinterpret a UInt as an Int (deprecated) + 0x2b ``as_uint`` ``( Int -> UInt)`` reinterpret an Int as a UInt (deprecated) 0x2c ``is_null`` ``( Object -> UInt )`` check an object for null ``(object ? 0 : 1)`` ======== ============= ================================================================ ``` ### Arithmetic, logic, and comparison operations -All of these operations are only defined for `Int` and `UInt` and both operands need to be of the same type. The `>>` operator is an arithmetic shift if the parameters are of type `Int`, otherwise it's a logical shift to the right. +Every `Integer` value on the data stack is signed. `+`, `-`, `*`, `/`, `%`, `=`, `!=`, `<`, `>`, `=<`, `>=` are defined for `Integer` (and deprecated `Int`/`UInt`) operate on the operands' mathematical values. + +`<<`, `>>`, `&`, `|`, `^`, `~` are bitwise operations and operate on an `Integer`'s underlying two's complement bit pattern rather than its mathematical value. Because a bitwise operation never treats its operands as having a sign, `>>` is always a logical (zero-filling) shift, not an arithmetic shift. ```{eval-rst} ======== ========== =========================== @@ -181,6 +185,7 @@ Most instructions are just a single byte opcode. The only exceptions are the lit - *String*: Length in bytes encoded as ULEB128, followed length bytes - *Int*: LEB128 - *UInt*: ULEB128 +- *Integer*: LEB128, sign-extended to a signed value of at least 64 bits - *Selector*: ULEB128 ### Embedding diff --git a/lldb/include/lldb/DataFormatters/FormatterBytecode.def b/lldb/include/lldb/DataFormatters/FormatterBytecode.def index 5556eef450e31..a0d2f93b07ab3 100644 --- a/lldb/include/lldb/DataFormatters/FormatterBytecode.def +++ b/lldb/include/lldb/DataFormatters/FormatterBytecode.def @@ -29,13 +29,14 @@ DEFINE_OPCODE(0x11, "if", if) DEFINE_OPCODE(0x12, "ifelse", ifelse) DEFINE_OPCODE(0x13, "return", return) -DEFINE_OPCODE(0x20, nullptr, lit_uint) -DEFINE_OPCODE(0x21, nullptr, lit_int) +DEFINE_OPCODE(0x20, nullptr, lit_uint) // Deprecated: use lit_integer. +DEFINE_OPCODE(0x21, nullptr, lit_int) // Deprecated: use lit_integer. DEFINE_OPCODE(0x22, nullptr, lit_string) DEFINE_OPCODE(0x23, nullptr, lit_selector) +DEFINE_OPCODE(0x24, nullptr, lit_integer) -DEFINE_OPCODE(0x2a, "as_int", as_int) -DEFINE_OPCODE(0x2b, "as_uint", as_uint) +DEFINE_OPCODE(0x2a, "as_int", as_int) // Deprecated. +DEFINE_OPCODE(0x2b, "as_uint", as_uint) // Deprecated. DEFINE_OPCODE(0x2c, "is_null", is_null) DEFINE_OPCODE(0x30, "+", plus) diff --git a/lldb/include/lldb/DataFormatters/FormatterBytecode.h b/lldb/include/lldb/DataFormatters/FormatterBytecode.h index 5838e8975ee4d..94b73f7c2ce07 100644 --- a/lldb/include/lldb/DataFormatters/FormatterBytecode.h +++ b/lldb/include/lldb/DataFormatters/FormatterBytecode.h @@ -11,12 +11,22 @@ #include "lldb/DataFormatters/TypeSummary.h" #include "lldb/Symbol/CompilerType.h" +#include "llvm/ADT/APSInt.h" namespace lldb_private { namespace FormatterBytecode { -enum DataType : uint8_t { Any, String, Int, UInt, Object, Type, Selector }; +enum DataType : uint8_t { + Any, + String, + Int, // Deprecated: use Integer. + UInt, // Deprecated: use Integer. + Object, + Type, + Selector, + Integer, +}; enum OpCodes : uint8_t { #define DEFINE_OPCODE(OP, MNEMONIC, NAME) op_##NAME = OP, @@ -38,9 +48,11 @@ enum Signatures : uint8_t { using ControlStackElement = llvm::StringRef; using ControlStack = std::vector<ControlStackElement>; +// uint64_t and int64_t are kept for compatibility with the deprecated +// uint/int opcodes. New code should instead use APSInt (op_lit_integer). using DataStackElement = std::variant<std::string, uint64_t, int64_t, lldb::ValueObjectSP, - CompilerType, Selectors>; + CompilerType, Selectors, llvm::APSInt>; struct DataStack : public std::vector<DataStackElement> { DataStack() = default; DataStack(lldb::ValueObjectSP initial_value) diff --git a/lldb/source/DataFormatters/FormatterBytecode.cpp b/lldb/source/DataFormatters/FormatterBytecode.cpp index 5185aea3b6d32..7775e455f88ef 100644 --- a/lldb/source/DataFormatters/FormatterBytecode.cpp +++ b/lldb/source/DataFormatters/FormatterBytecode.cpp @@ -68,6 +68,8 @@ std::string toString(const FormatterBytecode::DataStack &data) { os << *u << 'u'; else if (auto i = std::get_if<int64_t>(&d)) os << *i; + else if (auto ap = std::get_if<llvm::APSInt>(&d)) + os << *ap; else if (auto valobj = std::get_if<ValueObjectSP>(&d)) { if (!valobj->get()) os << "null"; @@ -119,6 +121,8 @@ static llvm::Error FormatImpl(DataStack &data) { format(FormatFunctor(u)); else if (auto i = std::get_if<int64_t>(&arg)) format(FormatFunctor(i)); + else if (auto ap = std::get_if<llvm::APSInt>(&arg)) + format(FormatFunctor(*ap)); else if (auto valobj = std::get_if<ValueObjectSP>(&arg)) { if (!valobj->get()) format(FormatFunctor("null object")); @@ -166,6 +170,10 @@ static llvm::Error TypeCheck(llvm::ArrayRef<DataStackElement> data, if (!std::holds_alternative<Selectors>(elem)) return llvm::createStringError("expected Selector"); break; + case Integer: + if (!std::holds_alternative<llvm::APSInt>(elem)) + return llvm::createStringError("expected Integer"); + break; } return llvm::Error::success(); } @@ -184,6 +192,19 @@ static llvm::Error TypeCheck(llvm::ArrayRef<DataStackElement> data, return TypeCheck(data.drop_back(1), type2, type1); } +/// Wrap the result of a binary operator applied to two APSInts back into a +/// DataStackElement. Comparison operators yield bool and need bit_width/ +/// is_unsigned to construct the boolean's APSInt representation; arithmetic +/// operators already yield a correctly-tagged APSInt and ignore them. +template <typename T> +static DataStackElement WrapAPSIntResult(T result, unsigned bit_width, + bool is_unsigned) { + if constexpr (std::is_same_v<T, bool>) + return llvm::APSInt(llvm::APInt(bit_width, result), is_unsigned); + else + return DataStackElement(std::move(result)); +} + llvm::Error Interpret(ControlStack &control, DataStack &data, Signatures sig) { if (control.empty()) return llvm::Error::success(); @@ -290,24 +311,40 @@ llvm::Error Interpret(ControlStack &control, DataStack &data, Signatures sig) { control.push_back(block); continue; } - case op_if: - TYPE_CHECK(UInt); - if (data.Pop<uint64_t>() != 0) { + case op_if: { + auto cond = data.PopAny(); + bool truthy; + if (auto *ap = std::get_if<llvm::APSInt>(&cond)) + truthy = !ap->isZero(); + else if (auto *u = std::get_if<uint64_t>(&cond)) + truthy = *u != 0; + else + return error("expected Integer or UInt"); + if (truthy) { if (!cur_block.size()) return error("empty control stack"); activate_block(); } else control.pop_back(); continue; - case op_ifelse: - TYPE_CHECK(UInt); + } + case op_ifelse: { if (cur_block.size() < 2) return error("empty control stack"); - if (data.Pop<uint64_t>() == 0) + auto cond = data.PopAny(); + bool truthy; + if (auto *ap = std::get_if<llvm::APSInt>(&cond)) + truthy = !ap->isZero(); + else if (auto *u = std::get_if<uint64_t>(&cond)) + truthy = *u != 0; + else + return error("expected Integer or UInt"); + if (!truthy) control[control.size() - 2] = control.back(); control.pop_back(); activate_block(); continue; + } case op_return: control.clear(); return pc.takeError(); @@ -319,6 +356,9 @@ llvm::Error Interpret(ControlStack &control, DataStack &data, Signatures sig) { case op_lit_int: data.Push(cur_block.getSLEB128(pc)); continue; + case op_lit_integer: + data.Push(cur_block.getSLEB128APSInt(pc)); + continue; case op_lit_selector: data.Push(Selectors(cur_block.getU8(pc))); continue; @@ -350,7 +390,7 @@ llvm::Error Interpret(ControlStack &control, DataStack &data, Signatures sig) { continue; } - // Arithmetic, logic, etc. +// Arithmetic operations. #define BINOP_IMPL(OP, CHECK_ZERO) \ { \ TYPE_CHECK(Any, Any); \ @@ -365,11 +405,75 @@ llvm::Error Interpret(ControlStack &control, DataStack &data, Signatures sig) { return error(#OP " by zero"); \ TYPE_CHECK(Int); \ data.Push((int64_t)(data.Pop<int64_t>() OP std::get<int64_t>(y))); \ + } else if (std::holds_alternative<llvm::APSInt>(y)) { \ + TYPE_CHECK(Integer); \ + llvm::APSInt rhs = std::get<llvm::APSInt>(y); \ + llvm::APSInt lhs = data.Pop<llvm::APSInt>(); \ + if (lhs.getBitWidth() != rhs.getBitWidth()) \ + return error("bit width mismatch"); \ + if (lhs.isUnsigned() != rhs.isUnsigned()) \ + return error("signedness mismatch"); \ + if (CHECK_ZERO && rhs.isZero()) \ + return error(#OP " by zero"); \ + data.Push(WrapAPSIntResult(lhs OP rhs, lhs.getBitWidth(), \ + lhs.isUnsigned())); \ } else \ return error("unsupported data types"); \ } #define BINOP(OP) BINOP_IMPL(OP, false) #define BINOP_CHECKZERO(OP) BINOP_IMPL(OP, true) + +// Comparision operations. +#define CMPOP(OP) \ + { \ + TYPE_CHECK(Any, Any); \ + auto y = data.PopAny(); \ + if (std::holds_alternative<uint64_t>(y)) { \ + TYPE_CHECK(UInt); \ + data.Push((uint64_t)(data.Pop<uint64_t>() OP std::get<uint64_t>(y))); \ + } else if (std::holds_alternative<int64_t>(y)) { \ + TYPE_CHECK(Int); \ + data.Push((int64_t)(data.Pop<int64_t>() OP std::get<int64_t>(y))); \ + } else if (std::holds_alternative<llvm::APSInt>(y)) { \ + TYPE_CHECK(Integer); \ + llvm::APSInt rhs = std::get<llvm::APSInt>(y); \ + llvm::APSInt lhs = data.Pop<llvm::APSInt>(); \ + if (lhs.getBitWidth() != rhs.getBitWidth()) \ + return error("bit width mismatch"); \ + if (lhs.isUnsigned() != rhs.isUnsigned()) \ + return error("signedness mismatch"); \ + data.Push(WrapAPSIntResult(lhs OP rhs, lhs.getBitWidth(), \ + lhs.isUnsigned())); \ + } else \ + return error("unsupported data types"); \ + } + +// Bitwise operations use an Integer's underlying bit pattern, not its +// mathematical value (ie signed-ness is ignored). This means >> is always a +// logical (zero-filling) shift, never an arithmetic shift. +#define BITOP(OP) \ + { \ + TYPE_CHECK(Any, Any); \ + auto y = data.PopAny(); \ + if (std::holds_alternative<uint64_t>(y)) { \ + TYPE_CHECK(UInt); \ + data.Push((uint64_t)(data.Pop<uint64_t>() OP std::get<uint64_t>(y))); \ + } else if (std::holds_alternative<int64_t>(y)) { \ + TYPE_CHECK(Int); \ + data.Push((int64_t)(data.Pop<int64_t>() OP std::get<int64_t>(y))); \ + } else if (std::holds_alternative<llvm::APSInt>(y)) { \ + TYPE_CHECK(Integer); \ + llvm::APSInt rhs = std::get<llvm::APSInt>(y); \ + llvm::APSInt lhs = data.Pop<llvm::APSInt>(); \ + if (lhs.getBitWidth() != rhs.getBitWidth()) \ + return error("bit width mismatch"); \ + llvm::APInt bits = static_cast<const llvm::APInt &>(lhs) \ + OP static_cast<const llvm::APInt &>(rhs); \ + data.Push(llvm::APSInt(std::move(bits), /*isUnsigned=*/false)); \ + } else \ + return error("unsupported data types"); \ + } + case op_plus: BINOP(+); continue; @@ -402,6 +506,14 @@ llvm::Error Interpret(ControlStack &control, DataStack &data, Signatures sig) { if (y > 64) \ return error("shift out of bounds"); \ data.Push(x OP y); \ + } else if (std::holds_alternative<llvm::APSInt>(data.back())) { \ + llvm::APSInt x = data.Pop<llvm::APSInt>(); \ + if (y > x.getBitWidth()) \ + return error("shift out of bounds"); \ + const llvm::APInt &bits = x; \ + llvm::APInt shifted = \ + LEFT ? bits.shl((unsigned)y) : bits.lshr((unsigned)y); \ + data.Push(llvm::APSInt(std::move(shifted), /*isUnsigned=*/false)); \ } else \ return error("unsupported data types"); \ } @@ -411,35 +523,43 @@ llvm::Error Interpret(ControlStack &control, DataStack &data, Signatures sig) { SHIFTOP(>>, false); continue; case op_and: - BINOP(&); + BITOP(&); continue; case op_or: - BINOP(|); + BITOP(|); continue; case op_xor: - BINOP(^); + BITOP(^); continue; - case op_not: - TYPE_CHECK(UInt); - data.Push(~data.Pop<uint64_t>()); + case op_not: { + TYPE_CHECK(Any); + auto x = data.PopAny(); + if (std::holds_alternative<uint64_t>(x)) + data.Push(~std::get<uint64_t>(x)); + else if (auto *ap = std::get_if<llvm::APSInt>(&x)) { + llvm::APInt bits = ~static_cast<const llvm::APInt &>(*ap); + data.Push(llvm::APSInt(std::move(bits), /*isUnsigned=*/false)); + } else + return error("unsupported data types"); continue; + } case op_eq: - BINOP(==); + CMPOP(==); continue; case op_neq: - BINOP(!=); + CMPOP(!=); continue; case op_lt: - BINOP(<); + CMPOP(<); continue; case op_gt: - BINOP(>); + CMPOP(>); continue; case op_le: - BINOP(<=); + CMPOP(<=); continue; case op_ge: - BINOP(>=); + CMPOP(>=); continue; case op_call: { TYPE_CHECK(Selector); diff --git a/lldb/source/DataFormatters/TypeSummary.cpp b/lldb/source/DataFormatters... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/220665 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
