mib marked 3 inline comments as done. mib added inline comments.
================ Comment at: lldb/source/Expression/DWARFExpression.cpp:936 /// Insertion point for evaluating multi-piece expression. - uint64_t op_piece_offset = 0; - Value pieces; // Used for DW_OP_piece + llvm::BitVector bit_pieces; + llvm::BitVector bit_mask; ---------------- aprantl wrote: > Is this only going to be used for DW_OP_bit_piece? Otherwise I would have > expected a name like `top(_of_stack)` or `result`. Yes, the same BitVector is used for both but I separated the DW_OP_piece changes for this diff, I forgot to change the name —‘ ================ Comment at: lldb/source/Expression/DWARFExpression.cpp:2505 + Value pieces; + pieces.AppendBytes(bit_pieces.getData().data(), bit_pieces.size() / 8); result = pieces; ---------------- vsk wrote: > Can we assert that this doesn't drop any bits (bit_pieces.size() % 8 == 0)? Actually, I think this should be changed to `std::ceil(bit_pieces.size() / 8.0)` instead of putting an assert ================ Comment at: lldb/source/Utility/Scalar.cpp:200 case e_uint512: - return (m_integer.getBitWidth() / 8); + return ceil(m_integer.getBitWidth() / 8.0); case e_float: ---------------- vsk wrote: > aprantl wrote: > > ? > This is worth a comment, I'm not sure what this is for. When the bitWidth is < 8, the division result is 0, but it should be 1 byte instead. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76449/new/ https://reviews.llvm.org/D76449 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits