From: Guinevere Larsen <guinevere@redhat.com>
To: Ezra.Sitorus@arm.com, gdb-patches@sourceware.org
Cc: luis.machado.foss@gmail.com, thiago.bauermann@linaro.org
Subject: Re: [PATCH 4/4] gdb/aarch64: record/replay support for LSE128
Date: Fri, 24 Apr 2026 14:12:25 -0300 [thread overview]
Message-ID: <8eff7331-d77d-400a-a1e0-5bde5e9219c0@redhat.com> (raw)
In-Reply-To: <20260420215232.68675-5-Ezra.Sitorus@arm.com>
On 4/20/26 6:52 PM, Ezra.Sitorus@arm.com wrote:
> From: Ezra Sitorus <ezra.sitorus@arm.com>
>
> FEAT_LSE128 introduces support for 128-bit atomic instructions. This
> patch teaches GDB to decode these instructions for recording and
> reversing.
>
> Regression tested on aarch64-none-linux-gnu on QEMU with LSE128 support.
> I observe unrelated regressions that appear to be environmental rather
> than caused by this patch.
Hi Ezra! Thanks for working on this :)
Once again, my only feedback is the use of the register keyword
> ---
> gdb/aarch64-tdep.c | 13 ++
> gdb/testsuite/gdb.reverse/aarch64-lse128.c | 139 +++++++++++++++++++
> gdb/testsuite/gdb.reverse/aarch64-lse128.exp | 138 ++++++++++++++++++
> gdb/testsuite/lib/gdb.exp | 56 ++++++++
> 4 files changed, 346 insertions(+)
> create mode 100644 gdb/testsuite/gdb.reverse/aarch64-lse128.c
> create mode 100644 gdb/testsuite/gdb.reverse/aarch64-lse128.exp
>
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 61c462979b3..aff8b294bd5 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -6004,6 +6004,19 @@ aarch64_record_load_store (aarch64_insn_decode_record *aarch64_insn_r)
> else if ((insn_bits24_27 & 1) == 1 && insn_bits28_29 == 1
> && insn_bits10_11 == 1 && !insn_bit21)
> return aarch64_record_memcopy_memset (aarch64_insn_r);
> + /* Large System Extension 128 (LSE128) instructions. */
> + else if (vector_flag == 0 && insn_bits10_11 == 0 && insn_bit21
> + && size_bits == 0 && !bit (aarch64_insn_r->aarch64_insn, 29)
> + && bits (aarch64_insn_r->aarch64_insn, 24, 25) == 0x1)
> + {
> + regcache_raw_read_unsigned (aarch64_insn_r->regcache, reg_rn, &address);
> + record_buf_mem[0] = 128 >> 3;
> + record_buf_mem[1] = address;
> + aarch64_insn_r->mem_rec_count = 1;
> + record_buf[0] = reg_rt;
> + record_buf[1] = bits (aarch64_insn_r->aarch64_insn, 16, 20);
> + aarch64_insn_r->reg_rec_count = 2;
> + }
> /* Advanced SIMD load/store instructions. */
> else
> return aarch64_record_asimd_load_store (aarch64_insn_r);
> diff --git a/gdb/testsuite/gdb.reverse/aarch64-lse128.c b/gdb/testsuite/gdb.reverse/aarch64-lse128.c
> new file mode 100644
> index 00000000000..906c303d904
> --- /dev/null
> +++ b/gdb/testsuite/gdb.reverse/aarch64-lse128.c
> @@ -0,0 +1,139 @@
> +/* This test program is part of GDB, the GNU debugger.
> +
> + Copyright 2024-2026 Free Software Foundation, Inc.
> +
> + 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 <http://www.gnu.org/licenses/>. */
> +
> +#include <stdint.h>
> +#include <stdalign.h>
> +#include <string.h>
> +
> +#define INITIAL_STRING "This is just some string."
> +#define BUF_SIZE sizeof (INITIAL_STRING)
> +
> +#define PREPARE_REGS(X, Y) \
> + strcpy (src, INITIAL_STRING); \
> + __asm__ volatile ("mov %0, %1\n" : "=r"(xa): "r"(X):); \
> + __asm__ volatile ("mov %0, %1\n" : "=r"(xb): "r"(Y):); \
> + ptr = (uint64_t *)src; \
> +
> +int
> +main (void)
> +{
> + alignas(16) char src[BUF_SIZE];
> +
> + register uint64_t xa asm ("x19");
> + register uint64_t xb asm ("x20");
> + register uint64_t *ptr asm ("x21");
> + uint64_t a = 0x0123456789abcdef;
> + uint64_t b = 0xfedbca9876543210;
> +
> + PREPARE_REGS (a, b);
> + /* Before ldclrp. */
> + __asm__ volatile ("ldclrp %0, %1, [%2]\n"
> + : "+r"(xa), "+r"(xb)
> + : "r"(ptr)
> + : "memory");
> + /* After ldclrp. */
> +
> + PREPARE_REGS (a, b);
> + /* Before ldclrpa. */
> + __asm__ volatile ("ldclrpa %0, %1, [%2]\n"
> + : "+r"(xa), "+r"(xb)
> + : "r"(ptr)
> + : "memory");
> + /* After ldclrpa. */
> +
> + PREPARE_REGS (a, b);
> + /* Before ldclrpal. */
> + __asm__ volatile ("ldclrpal %0, %1, [%2]\n"
> + : "+r"(xa), "+r"(xb)
> + : "r"(ptr)
> + : "memory");
> + /* After ldclrpal. */
> +
> + PREPARE_REGS (a, b);
> + /* Before ldclrpl. */
> + __asm__ volatile ("ldclrpl %0, %1, [%2]\n"
> + : "+r"(xa), "+r"(xb)
> + : "r"(ptr)
> + : "memory");
> + /* After ldclrpl. */
> +
> + PREPARE_REGS (a, b);
> + /* Before ldsetp. */
> + __asm__ volatile ("ldsetp %0, %1, [%2]\n"
> + : "+r"(xa), "+r"(xb)
> + : "r"(ptr)
> + : "memory");
> + /* After ldsetp. */
> +
> + PREPARE_REGS (a, b);
> + /* Before ldsetpa. */
> + __asm__ volatile ("ldsetpa %0, %1, [%2]\n"
> + : "+r"(xa), "+r"(xb)
> + : "r"(ptr)
> + : "memory");
> + /* After ldsetpa. */
> +
> + PREPARE_REGS (a, b);
> + /* Before ldsetpal. */
> + __asm__ volatile ("ldsetpal %0, %1, [%2]\n"
> + : "+r"(xa), "+r"(xb)
> + : "r"(ptr)
> + : "memory");
> + /* After ldsetpal. */
> +
> + PREPARE_REGS (a, b);
> + /* Before ldsetpl. */
> + __asm__ volatile ("ldsetpl %0, %1, [%2]\n"
> + : "+r"(xa), "+r"(xb)
> + : "r"(ptr)
> + : "memory");
> + /* After ldsetpl. */
> +
> + PREPARE_REGS (a, b);
> + /* Before swpp. */
> + __asm__ volatile ("swpp %0, %1, [%2]\n"
> + : "+r"(xa), "+r"(xb)
> + : "r"(ptr)
> + : "memory");
> + /* After swpp. */
> +
> + PREPARE_REGS (a, b);
> + /* Before swppa. */
> + __asm__ volatile ("swppa %0, %1, [%2]\n"
> + : "+r"(xa), "+r"(xb)
> + : "r"(ptr)
> + : "memory");
> + /* After swppa. */
> +
> + PREPARE_REGS (a, b);
> + /* Before swppal. */
> + __asm__ volatile ("swppal %0, %1, [%2]\n"
> + : "+r"(xa), "+r"(xb)
> + : "r"(ptr)
> + : "memory");
> + /* After swppal. */
> +
> + PREPARE_REGS (a, b);
> + /* Before swppl. */
> + __asm__ volatile ("swppl %0, %1, [%2]\n"
> + : "+r"(xa), "+r"(xb)
> + : "r"(ptr)
> + : "memory");
> + /* After swppl. */
> +
> + return 0;
> +}
> diff --git a/gdb/testsuite/gdb.reverse/aarch64-lse128.exp b/gdb/testsuite/gdb.reverse/aarch64-lse128.exp
> new file mode 100644
> index 00000000000..273e239ec66
> --- /dev/null
> +++ b/gdb/testsuite/gdb.reverse/aarch64-lse128.exp
> @@ -0,0 +1,138 @@
> +# Copyright 2024-2026 Free Software Foundation, Inc.
> +
> +# 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 <http://www.gnu.org/licenses/>.
> +
> +# Test instruction record for AArch64 FEAT_LSE128 instructions.
> +# Based on gdb.reverse/aarch64-mops.exp
> +#
> +# The basic flow of the record tests are:
> +# 1) Stop before executing the instructions of interest. Record
> +# the initial value of the registers that the instruction will
> +# change, i.e. the destination register.
> +# 2) Execute the instructions. Record the new value of the
> +# registers that changed.
> +# 3) Reverse the direction of the execution and execute back to
> +# just before the instructions of interest. Record the final
> +# value of the registers of interest.
> +# 4) Check that the initial and new values of the registers are
> +# different, i.e. the instruction changed the registers as expected.
> +# 5) Check that the initial and final values of the registers are
> +# the same, i.e. GDB record restored the registers to their
> +# original values.
> +
> +standard_testfile
> +
> +require allow_aarch64_lse128_tests
> +
> +if {
> + [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
> + [list debug additional_flags=-march=armv8-a+lse128]]
> +} {
> + return -1
> +}
> +
> +if {![runto_main]} {
> + return -1
> +}
> +
> +gdb_test_no_output "record full"
> +
> +proc test_single_asm {insn} {
> + global decimal hex
> +
> + set before_seq [gdb_get_line_number "Before ${insn}"]
> + set after_seq [gdb_get_line_number "After ${insn}"]
> + set regs { x19 x20 }
> +
> + gdb_test "break $before_seq" \
> + "Breakpoint $decimal at $hex: file .*/aarch64-lse128.c, line $decimal\\." \
> + "$insn: break before instruction sequence"
> + gdb_continue_to_breakpoint "$insn: about to execute instruction sequence" \
> + [multi_line ".*/aarch64-lse128.c:$decimal" \
> + "$decimal\[ \t\]+__asm__ volatile \\(\"${insn} \[^\r\n\]+\""]
> +
> + # Depending on the compiler, the line number information may put GDB a few
> + # instructions before the beginning of the asm statement.
> + arrive_at_instruction $insn
> + # Add a breakpoint that we're sure is at the prologue instruction.
> + gdb_test "break *\$pc" \
> + "Breakpoint $decimal at $hex: file .*/aarch64-lse128.c, line $decimal\\." \
> + "$insn: break at prologue instruction"
> +
> + # Record the initial memory and register values.
> + foreach r $regs {
> + set ${r}_initial [capture_command_output "info register $r" ""]
> + }
> + set ptr_initial [capture_command_output "x/4gwx ptr" ""]
> +
> + gdb_test "break $after_seq" \
> + "Breakpoint $decimal at $hex: file .*/aarch64-lse128.c, line $decimal\\." \
> + "$insn: break after instruction sequence"
> + gdb_continue_to_breakpoint "$insn: executed instruction sequence" \
> + [multi_line ".*/aarch64-lse128.c:$decimal" ".*"]
> +
> + # Record the new memory and register values.
> + foreach r $regs {
> + set ${r}_new [capture_command_output "info register $r" ""]
> + }
> + set ptr_new [capture_command_output "x/4gwx ptr" ""]
> +
> + # Execute in reverse to before the instruction sequence.
> + gdb_test_no_output "set exec-direction reverse"
> +
> + gdb_continue_to_breakpoint "reversed execution of instruction sequence" \
> + [multi_line ".*/aarch64-lse128.c:$decimal" \
> + "$decimal\[ \t\]+__asm__ volatile \\(\"${insn} \[^\r\n\]+\""]
> +
> + # Record the final memory and register values.
> + foreach r $regs {
> + set ${r}_final [capture_command_output "info register $r" ""]
> + }
> + set ptr_final [capture_command_output "x/4gwx ptr" ""]
> +
> + foreach v [concat $regs {ptr}] {
> + gdb_assert ![string compare [set ${v}_initial] [set ${v}_final]] \
> + "$insn: check $v initial value versus $v final value"
> + gdb_assert [string compare [set ${v}_initial] [set ${v}_new]] \
> + "$insn: check $v initial value versus $v new value"
> + }
> +
> + # Restore forward execution and go to end of recording.
> + gdb_test_no_output "set exec-direction forward"
> + gdb_test "record goto end" \
> + [multi_line \
> + "Go forward to insn number $decimal" \
> + "#0 main \\(\\) at .*/aarch64-lse128.c:$decimal" \
> + ".*"]
> +}
> +
> +set cases {
> + { ldclrp }
> + { ldclrpa }
> + { ldclrpal }
> + { ldclrpl }
> + { ldsetp }
> + { ldsetpa }
> + { ldsetpal }
> + { ldsetpl }
> + { swpp }
> + { swppa }
> + { swppal }
> + { swppl }
> +}
> +
> +foreach c $cases {
> + lassign $c insn
> + test_single_asm $insn
> +}
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 7d400e9028a..d5f9008470a 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -5402,6 +5402,62 @@ gdb_caching_proc allow_aarch64_cssc_tests {} {
> return $allow_cssc_tests
> }
>
> +# Run a test on the target to see if it supports the AArch64 LSE128 feature.
> +# Return 1 if so, 0 if it does not. Note this causes a restart of GDB.
> +
> +gdb_caching_proc allow_aarch64_lse128_tests {} {
> + global srcdir subdir gdb_prompt inferior_exited_re
> +
> + set me "allow_aarch64_lse128_tests"
> +
> + if { ![is_aarch64_target]} {
> + return 0
> + }
> +
> + set compile_flags "{additional_flags=-march=armv8-a+lse128}"
> +
> + # Compile a test program reading LSE128.
> + set src {
> + #include <stdint.h>
> +
> + int main() {
> + __attribute__((aligned(16))) uint64_t mem[2] = { 0x0, 0x1 };
> + uint64_t *ptr = mem;
> + __asm__ volatile ("ldclrp x0, x1, [%0]\n" :: "r"(ptr) : "x0", "x1", "memory");
> + return 0;
> + }
> + }
> +
> + if {![gdb_simple_compile $me $src executable $compile_flags]} {
> + return 0
> + }
> +
> + # Compilation succeeded so now run it via gdb.
> + clean_restart
> + gdb_load $obj
> + gdb_run_cmd
> +
> + gdb_expect {
> + -re ".*Illegal instruction.*${gdb_prompt} $" {
> + verbose -log "\n$me lse128 support not detected"
> + set allow_lse128_tests 0
> + }
> + -re ".*$inferior_exited_re normally.*${gdb_prompt} $" {
> + verbose -log "\n$me: lse128 support detected"
> + set allow_lse128_tests 1
> + }
> + default {
> + warning "\n$me: default case taken"
> + set allow_lse128_tests 0
> + }
> + }
> + gdb_exit
> + remote_file build delete $obj
> +
> + verbose "$me: returning $allow_lse128_tests" 2
> + return $allow_lse128_tests
> +}
> +
> # Run a test on the target to see if it supports AArch64 MOPS (Memory
> # Operations) extensions. Return 1 if so, 0 if it does not. Note this
> # causes a restart of GDB.
--
Cheers,
Guinevere Larsen
It/she
next prev parent reply other threads:[~2026-04-24 17:13 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-20 21:52 [PATCH 0/4] gdb/aarch64: record/replay support for AArch64 Ezra.Sitorus
2026-04-20 21:52 ` [PATCH 1/4] gdb/aarch64: record/replay support for LRCPC3 Ezra.Sitorus
2026-04-24 7:00 ` Luis
2026-04-24 15:09 ` Guinevere Larsen
2026-04-25 5:08 ` Thiago Jung Bauermann
2026-04-25 7:21 ` Thiago Jung Bauermann
2026-04-20 21:52 ` [PATCH 2/4] gdb/aarch64: Test record/replay support for CSSC Ezra.Sitorus
2026-04-24 7:07 ` Luis
2026-04-24 16:53 ` Guinevere Larsen
2026-04-25 6:10 ` Thiago Jung Bauermann
2026-04-25 7:28 ` Thiago Jung Bauermann
2026-04-20 21:52 ` [PATCH 3/4] gdb/aarch64: record/replay support for RPRFM and PRFM (reg) Ezra.Sitorus
2026-04-24 7:08 ` Luis
2026-04-24 16:56 ` Guinevere Larsen
2026-04-25 8:12 ` Thiago Jung Bauermann
2026-04-20 21:52 ` [PATCH 4/4] gdb/aarch64: record/replay support for LSE128 Ezra.Sitorus
2026-04-24 7:11 ` Luis
2026-04-24 17:12 ` Guinevere Larsen [this message]
2026-04-25 8:38 ` Thiago Jung Bauermann
2026-04-23 17:31 ` [PATCH 0/4] gdb/aarch64: record/replay support for AArch64 Guinevere Larsen
2026-04-23 22:20 ` Ezra Sitorus
2026-04-24 5:33 ` Thiago Jung Bauermann
2026-04-24 7:16 ` Luis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8eff7331-d77d-400a-a1e0-5bde5e9219c0@redhat.com \
--to=guinevere@redhat.com \
--cc=Ezra.Sitorus@arm.com \
--cc=gdb-patches@sourceware.org \
--cc=luis.machado.foss@gmail.com \
--cc=thiago.bauermann@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox