Re: [PATCH v2 05/47] gdb, arch, intelgt: add intelgt arch definitions

Thiago Jung Bauermann
2025-07-08T03:03:39Z

Tankut Baris Aktemur <tankut.baris.aktemur@intel.com> writes:

> From: Markus Metzger <markus.t.metzger@intel.com>
>
> Provide Intel GT architecture-specific definitions that can be used by
> both the low target at the server side and tdep at the GDB side.
>
> Other than, for example, IA, Intel GT does not have a dedicated
> breakpoint instruction. Instead, it has a breakpoint bit in each
> instruction. We define arch methods for dealing with instruction
> breakpoint bits.
>
> Co-authored-by: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
> Co-authored-by: Mihails Strasuns <mihails.strasuns@intel.com>
> Co-authored-by: Natalia Saiapova <natalia.saiapova@intel.com>
> ---
> gdb/Makefile.in | 1 +
> gdb/arch/intelgt.c | 77 +++++++++++++++++++++++
> gdb/arch/intelgt.h | 175 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 253 insertions(+)

Just one nit below, but regardless:

Reviewed-By: Thiago Jung Bauermann <thiago.bauermann@linaro.org>

<snip>

> +static inline unsigned int
> +inst_length_compacted ()
> +{
> + return COMPACT_INST_LENGTH;
> +}
> +
> +static inline unsigned int
> +inst_length_full ()
> +{
> + return MAX_INST_LENGTH;
> +}

Not sure I see an advantage in these functions. I suggest removing them
and using the constants directly.

> +static inline unsigned int
> +inst_length (gdb::array_view<const gdb_byte> inst)
> +{
> + return (is_compacted_inst (inst)
> + ? inst_length_compacted ()
> + : inst_length_full ());
> +}
> +
> +} /* namespace intelgt */
> +
> +#endif

--
Thiago