From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lndn.lancelotsix.com (vps-42846194.vps.ovh.net [IPv6:2001:41d0:801:2000::2400]) by sourceware.org (Postfix) with ESMTPS id DDC883858403 for ; Mon, 1 Nov 2021 22:25:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DDC883858403 Received: from ubuntu.lan (unknown [IPv6:2a02:390:9086::635]) by lndn.lancelotsix.com (Postfix) with ESMTPSA id EBE88819D4; Mon, 1 Nov 2021 22:25:04 +0000 (UTC) Date: Mon, 1 Nov 2021 22:25:00 +0000 From: Lancelot SIX To: Zoran Zaric Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v3 28/28] Add DW_OP_LLVM_select_bit_piece DWARF operation Message-ID: <20211101222500.prqxzsx2hhg5pips@ubuntu.lan> References: <20211014093235.69756-1-zoran.zaric@amd.com> <20211014093235.69756-29-zoran.zaric@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20211014093235.69756-29-zoran.zaric@amd.com> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.11 (lndn.lancelotsix.com [0.0.0.0]); Mon, 01 Nov 2021 22:25:05 +0000 (UTC) X-Spam-Status: No, score=-9.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_SBL_CSS, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Nov 2021 22:25:10 -0000 Hi, I have included remarks below. On Thu, Oct 14, 2021 at 10:32:35AM +0100, Zoran Zaric via Gdb-patches wrote: > Second more complex DWARF expression operation, that is usefull for > SIMD/SIMT like architectures is DW_OP_LLVM_select_bit_piece. This > operation pops three stack entries, where the first must be an integral > type value that represents a bit mask, the second must be a location > description that represents the one-location description and the third > must be a location description that represents the zero-location > description. > > Resulting composite location description contains a given number of > pieces of a given bit size, created with parts from either of the two > location description, based on the bit mask. > > gdb/ChangeLog: > > * compile/compile-loc2c.c (compute_stack_depth_worker): Add > new DW_OP_LLVM_select_bit_piece operation support. > * dwarf2/expr.c (dwarf_expr_context::create_select_composite): > New method that creates the select bit piece composite. > (dwarf_location::slice): New method. > (dwarf_composite::slice): New method. > (dwarf_expr_context::execute_stack_op): Add new > DW_OP_LLVM_select_bit_piece operation support. > * dwarf2/loc.c (dwarf2_get_symbol_read_needs): Add new > DW_OP_LLVM_select_bit_piece operation support. > (disassemble_dwarf_expression): Add new > DW_OP_LLVM_select_bit_piece operation support. > > include/ChangeLog: > > * dwarf2.def: Add new DW_OP_LLVM_select_bit_piece enumeration. > > gdb/testsuite/ChangeLog: > > * gdb.dwarf2/dw2-llvm-select-bit-piece.exp: New test. > * lib/dwarf.exp: Add new DW_OP_LLVM_select_bit_piece operation > support. > --- > gdb/compile/compile-loc2c.c | 1 + > gdb/dwarf2/expr.c | 160 ++++++++++++++++++ > gdb/dwarf2/loc.c | 12 ++ > .../gdb.dwarf2/dw2-llvm-select-bit-piece.exp | 138 +++++++++++++++ > gdb/testsuite/lib/dwarf.exp | 5 + > include/dwarf2.def | 1 + > 6 files changed, 317 insertions(+) > create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-llvm-select-bit-piece.exp > > diff --git a/gdb/compile/compile-loc2c.c b/gdb/compile/compile-loc2c.c > index 2c5e13cfb6c..5991760adbc 100644 > --- a/gdb/compile/compile-loc2c.c > +++ b/gdb/compile/compile-loc2c.c > @@ -367,6 +367,7 @@ compute_stack_depth_worker (int start, int *need_tempvar, > break; > > case DW_OP_LLVM_extend: > + case DW_OP_LLVM_select_bit_piece: > stack_depth -= 2; > break; > > diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c > index 711a5918946..874a5d1923b 100644 > --- a/gdb/dwarf2/expr.c > +++ b/gdb/dwarf2/expr.c > @@ -461,6 +461,15 @@ class dwarf_location : public dwarf_entry > ill_formed_expression (); > } > > + /* Make a slice of a location description with an added bit offset > + BIT_OFFSET and BIT_SIZE size in bits. > + > + In the case of a composite location description, function returns > + a minimum subset of that location description that starts on a > + given offset of a given size. */ > + virtual std::unique_ptr slice (LONGEST bit_offset, > + LONGEST bit_size) const; > + > /* Read contents from the described location. > > The read operation is performed in the context of a FRAME. > @@ -621,6 +630,17 @@ const dwarf_location &computed_closure::get_location () const > return *m_location; > } > > +/* This is a default implementation used for > + non-composite location descriptions. */ > + > +std::unique_ptr > +dwarf_location::slice (LONGEST bit_offset, LONGEST bit_size) const > +{ > + dwarf_location_up location_slice = this->clone_location (); > + location_slice->add_bit_offset (bit_offset); > + return location_slice; > +} > + > void > dwarf_location::read_from_gdb_value (frame_info *frame, struct value *value, > int value_bit_offset, > @@ -1567,6 +1587,9 @@ class dwarf_composite final : public dwarf_location > return make_unique (*this); > } > > + std::unique_ptr slice (LONGEST bit_offset, > + LONGEST bit_size) const override; > + > void add_piece (std::unique_ptr location, ULONGEST bit_size) > { > gdb_assert (location != nullptr); > @@ -1653,6 +1676,64 @@ class dwarf_composite final : public dwarf_location > bool m_completed = false; > }; > > +std::unique_ptr > +dwarf_composite::slice (LONGEST bit_offset, LONGEST bit_size) const > +{ > + /* Size 0 is never expected at this point. */ > + gdb_assert (bit_size != 0); > + > + unsigned int pieces_num = m_pieces.size (); > + LONGEST total_bit_size = bit_size; > + LONGEST total_bits_to_skip = m_offset * HOST_CHAR_BIT > + + m_bit_suboffset + bit_offset; > + std::vector piece_slices; > + unsigned int i; > + > + for (i = 0; i < pieces_num; i++) > + { > + LONGEST piece_bit_size = m_pieces[i].size; > + > + if (total_bits_to_skip < piece_bit_size) > + break; > + > + total_bits_to_skip -= piece_bit_size; > + } > + > + for (; i < pieces_num; i++) > + { > + if (total_bit_size == 0) > + break; > + > + gdb_assert (total_bit_size > 0); > + LONGEST slice_bit_size = m_pieces[i].size - total_bits_to_skip; > + > + if (total_bit_size < slice_bit_size) > + slice_bit_size = total_bit_size; > + > + std::unique_ptr slice > + = m_pieces[i].location->slice (total_bits_to_skip, slice_bit_size); > + piece_slices.emplace_back (std::move (slice), slice_bit_size); > + > + total_bit_size -= slice_bit_size; > + total_bits_to_skip = 0; > + } > + > + unsigned int slices_num = piece_slices.size (); > + > + /* Only one piece found, so there is no reason to > + make a composite location description. */ > + if (slices_num == 1) > + return std::move (piece_slices[0].location); > + > + std::unique_ptr composite_slice > + = make_unique (m_arch, m_per_cu); > + > + for (piece &piece : piece_slices) > + composite_slice->add_piece (std::move (piece.location), piece.size); > + > + return composite_slice; > +} > + > void > dwarf_composite::read (frame_info *frame, gdb_byte *buf, > int buf_bit_offset, size_t bit_size, > @@ -2507,6 +2588,20 @@ struct dwarf_expr_context > void create_extend_composite (ULONGEST piece_bit_size, > ULONGEST pieces_count); > > + /* It pops three stack entries. The first must be an integral type > + value that represents a bit mask. The second must be a location > + description that represents the one-location description. The > + third must be a location description that represents the > + zero-location description. > + > + A complete composite location description created with parts from > + either of the two location description, based on the bit mask, > + is pushed on top of the DWARF stack. PIECE_BIT_SIZE represent > + a size in bits of each piece and PIECES_COUNT represents a number > + of pieces required. */ > + void create_select_composite (ULONGEST piece_bit_size, > + ULONGEST pieces_count); > + > /* The engine for the expression evaluator. Using the context in this > object, evaluate the expression between OP_PTR and OP_END. */ > void execute_stack_op (const gdb_byte *op_ptr, const gdb_byte *op_end); > @@ -2893,6 +2988,60 @@ dwarf_expr_context::create_extend_composite (ULONGEST piece_bit_size, > push (std::move (composite)); > } > > +void > +dwarf_expr_context::create_select_composite (ULONGEST piece_bit_size, > + ULONGEST pieces_count) > +{ > + gdb::byte_vector mask_buf; > + gdbarch *arch = this->m_per_objfile->objfile->arch (); > + > + if (stack_empty_p () || piece_bit_size == 0 || pieces_count == 0) > + ill_formed_expression (); > + > + dwarf_value_up mask = to_value (pop (), address_type ()); > + > + type *mask_type = mask->type (); > + ULONGEST mask_size = TYPE_LENGTH (mask_type); > + dwarf_require_integral (mask_type); > + > + if (mask_size * HOST_CHAR_BIT < pieces_count) > + ill_formed_expression (); > + > + mask_buf.resize (mask_size); > + > + copy_bitwise (mask_buf.data (), 0, mask->contents ().data (), > + 0, mask_size * HOST_CHAR_BIT, > + type_byte_order (mask_type) == BFD_ENDIAN_BIG); > + > + if (stack_empty_p ()) > + ill_formed_expression (); > + > + dwarf_location_up one = to_location (pop (), arch); > + > + if (stack_empty_p ()) > + ill_formed_expression (); > + > + dwarf_location_up zero = to_location (pop (), arch); > + > + std::unique_ptr composite > + = make_unique (arch, this->m_per_cu); > + > + for (ULONGEST i = 0; i < pieces_count; i++) > + { > + std::unique_ptr slice; > + > + if ((mask_buf.data ()[i / HOST_CHAR_BIT] >> (i % HOST_CHAR_BIT)) & 1) > + slice = one->slice (i * piece_bit_size, piece_bit_size); > + else > + slice = zero->slice (i * piece_bit_size, piece_bit_size); > + > + composite->add_piece (std::move (slice), piece_bit_size); > + } > + > + composite->set_completed (true); > + push (std::move (composite)); > +} > + > void > dwarf_expr_context::eval (const gdb_byte *addr, size_t len) > { > @@ -4134,6 +4283,17 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr, > break; > } > > + case DW_OP_LLVM_select_bit_piece: > + { > + uint64_t piece_bit_size, pieces_count; > + > + /* Record the piece. */ > + op_ptr = safe_read_uleb128 (op_ptr, op_end, &piece_bit_size); > + op_ptr = safe_read_uleb128 (op_ptr, op_end, &pieces_count); > + create_select_composite (piece_bit_size, pieces_count); > + break; > + } > + > default: > error (_("Unhandled dwarf expression opcode 0x%x"), op); > } > diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c > index de3a904695a..5ca9f525f39 100644 > --- a/gdb/dwarf2/loc.c > +++ b/gdb/dwarf2/loc.c > @@ -1954,6 +1954,7 @@ dwarf2_get_symbol_read_needs (gdb::array_view expr, > > case DW_OP_bit_piece: > case DW_OP_LLVM_extend: > + case DW_OP_LLVM_select_bit_piece: > op_ptr = safe_skip_leb128 (op_ptr, expr_end); > op_ptr = safe_skip_leb128 (op_ptr, expr_end); > break; > @@ -3678,6 +3679,17 @@ disassemble_dwarf_expression (struct ui_file *stream, > pulongest (ul), pulongest (count)); > } > break; > + > + case DW_OP_LLVM_select_bit_piece: > + { > + uint64_t count; > + > + data = safe_read_uleb128 (data, end, &ul); > + data = safe_read_uleb128 (data, end, &count); > + fprintf_filtered (stream, " piece size %s (bits) pieces count %s", > + pulongest (ul), pulongest (count)); > + } > + break; > } > > fprintf_filtered (stream, "\n"); > diff --git a/gdb/testsuite/gdb.dwarf2/dw2-llvm-select-bit-piece.exp b/gdb/testsuite/gdb.dwarf2/dw2-llvm-select-bit-piece.exp > new file mode 100644 > index 00000000000..c5ed6efbd75 > --- /dev/null > +++ b/gdb/testsuite/gdb.dwarf2/dw2-llvm-select-bit-piece.exp > @@ -0,0 +1,138 @@ > +# Copyright (C) 2017-2021 Free Software Foundation, Inc. > +# Copyright (C) 2020-2021 Advanced Micro Devices, Inc. All rights reserved. Based on a comment in the following thread: https://sourceware.org/pipermail/gdb-patches/2021-September/182104.html, I guess that this second line of copyright should be removed. I did not spot in the earlier patch, but it looks that the same line remains in gdb.dwarf2/dw2-llvm-extend.exp as well. > + > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see . > + > +# Test the new DW_OP_LLVM_select_bit_piece operation. > +# > +# The test uses a composite location description, where all the pieces > +# are selected from two registers in odd/even pattern using the new > +# operation. > + > +load_lib dwarf.exp > + > +# This test can only be run on targets which support DWARF-2 and use gas. > +if {![dwarf2_support]} { > + return 0 > +} > + > +# Choose suitable integer registers for the test. > + > +set dwarf_regnum {0 1} > + > +if { [is_aarch64_target] } { > + set regname {x0 x1} > +} elseif { [is_aarch32_target] > + || [istarget "s390*-*-*" ] > + || [istarget "powerpc*-*-*"] > + || [istarget "rs6000*-*-aix*"] } { > + set regname {r0 r1} > +} elseif { [is_x86_like_target] } { > + set regname {eax ecx} > +} elseif { [is_amd64_regs_target] } { > + set regname {rax rdx} > +} else { > + verbose "Skipping $gdb_test_file_name." > + return > +} > + > +standard_testfile var-access.c ${gdb_test_file_name}-dw.S > + > +# Make some DWARF for the test. > + > +set asm_file [standard_output_file $srcfile2] > +Dwarf::assemble $asm_file { > + global dwarf_regnum regname srcdir subdir srcfile > + set buf_src [gdb_target_symbol buf] > + > + set main_result [function_range main ${srcdir}/${subdir}/${srcfile}] > + set main_start [lindex $main_result 0] > + set main_length [lindex $main_result 1] > + > + cu {} { > + DW_TAG_compile_unit { > + {DW_AT_name var-access.c} > + {DW_AT_comp_dir /tmp} > + } { > + declare_labels int_type_label char_type_label array_type_label > + > + # define char type > + char_type_label: DW_TAG_base_type { > + {DW_AT_name "char"} > + {DW_AT_encoding @DW_ATE_signed} > + {DW_AT_byte_size 1 DW_FORM_sdata} > + } > + > + int_type_label: DW_TAG_base_type { > + {DW_AT_name "int"} > + {DW_AT_encoding @DW_ATE_signed} > + {DW_AT_byte_size 4 DW_FORM_sdata} > + } > + > + array_type_label: DW_TAG_array_type { > + {DW_AT_type :$char_type_label} > + } { > + DW_TAG_subrange_type { > + {DW_AT_type :$int_type_label} > + {DW_AT_upper_bound 7 DW_FORM_udata} > + } > + } > + > + DW_TAG_subprogram { > + {DW_AT_name main} > + {DW_AT_low_pc $main_start addr} > + {DW_AT_high_pc $main_length data8} > + } { > + > + # Odd array elements are in first byte of REGNAME 0 register, > + # while even elements are in first byte of REGNAME 1 register. > + DW_TAG_variable { > + {DW_AT_name var_array} > + {DW_AT_type :$array_type_label} > + {DW_AT_location { > + DW_OP_regx [lindex $dwarf_regnum 0] > + DW_OP_LLVM_extend 8 8 > + DW_OP_regx [lindex $dwarf_regnum 1] Looks like you have spaces instead of tabs to indent this line. Best, Lancelot. > + DW_OP_LLVM_extend 8 8 > + DW_OP_constu 0xaa > + DW_OP_LLVM_select_bit_piece 8 8 > + } SPECIAL_expr} > + } > + } > + } > + } > +} > + > +if { [prepare_for_testing ${testfile}.exp ${testfile} \ > + [list $srcfile $asm_file] {nodebug}] } { > + return -1 > +} > + > +if ![runto_main] { > + return -1 > +} > + > +gdb_test_no_output "set var \$[lindex $regname 0] = 0x04030201" "init reg 0" > +gdb_test_no_output "set var \$[lindex $regname 1] = 0x01020304" "init reg 1" > + > +# Determine byte order. > +set endian [get_endianness] > + > +switch $endian { > + little {set val "0x1, 0x4, 0x1, 0x4, 0x1, 0x4, 0x1, 0x4"} > + big {set val "0x4, 0x1, 0x4, 0x1, 0x4, 0x1, 0x4, 0x1"} > +} > + > +gdb_test "print/x var_array" " = \\{${val}\\}" "var_array print" > + > diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp > index 41415986e64..a8b9518a415 100644 > --- a/gdb/testsuite/lib/dwarf.exp > +++ b/gdb/testsuite/lib/dwarf.exp > @@ -1230,6 +1230,11 @@ namespace eval Dwarf { > _op .uleb128 [lindex $line 2] > } > > + DW_OP_LLVM_select_bit_piece { > + _op .uleb128 [lindex $line 1] > + _op .uleb128 [lindex $line 2] > + } > + > default { > if {[llength $line] > 1} { > error "Unimplemented: operands in location for $opcode" > diff --git a/include/dwarf2.def b/include/dwarf2.def > index 3d1d831d0fb..abc1125df2c 100644 > --- a/include/dwarf2.def > +++ b/include/dwarf2.def > @@ -711,6 +711,7 @@ DW_OP_DUP (DW_OP_LLVM_bit_offset, 0xe5) > DW_OP (DW_OP_LLVM_undefined, 0xe7) > DW_OP_DUP (DW_OP_LLVM_piece_end, 0xea) > DW_OP (DW_OP_LLVM_extend, 0xeb) > +DW_OP (DW_OP_LLVM_select_bit_piece, 0xec) > DW_END_OP > > DW_FIRST_ATE (DW_ATE_void, 0x0) > -- > 2.17.1 >