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

[Thiago Jung Bauermann <thiago.bauermann@linaro.org>]

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