Re: [PATCH v5 12/12] [PowerPC] Add support for HTM registers

[Simon Marchi <simark at simark dot ca>]

On 2018-10-22 6:32 p.m., Pedro Franco de Carvalho wrote:
>  /* Write method for POWER7 Extended FP pseudo-registers.  */
>  static void
> -efpr_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
> +efp_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
>  			    int reg_nr, const gdb_byte *buffer)
>  {
>    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> -  int reg_index = reg_nr - tdep->ppc_efpr0_regnum;
> +  int reg_index, vr0;
>    int offset = gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG ? 0 : 8;
>  
> +  if (IS_EFP_PSEUDOREG (tdep, reg_nr))
> +    {
> +      reg_index = reg_nr - tdep->ppc_efpr0_regnum;
> +      vr0 = PPC_VR0_REGNUM;
> +    }
> +  else
> +    {
> +      gdb_assert (IS_CEFP_PSEUDOREG (tdep, reg_nr));
> +
> +      reg_index = reg_nr - tdep->ppc_cefpr0_regnum;
> +      vr0 = PPC_CVR0_REGNUM;
> +
> +      /* The call to raw_write_part fails silently if the initial read
> +	 of the read-update-write sequence returns an invalid status,
> +	 so we check this manually and throw an error if needed.  */
> +      regcache->raw_update (vr0 + reg_index);
> +      if (regcache->get_register_status (vr0 + reg_index) != REG_VALID)
> +	error (_("Cannot write to the checkpointed EFP register, "
> +		 "the corresponding vector register is unavailable."));
> +    }
> +
>    /* Write the portion that overlaps the VMX register.  */
> -  regcache->raw_write_part (tdep->ppc_vr0_regnum + reg_index, offset,
> +  regcache->raw_write_part (vr0 + reg_index, offset,
>  			    register_size (gdbarch, reg_nr), buffer);
>  }

Hi Pedro,

I am trying to rebase this patch series [1], and I am having a small issue with
the code above.  I am introducing an abstraction layer on top of a regcache, so
that we are able to read and write registers from and to regcaches as well as from
and to frames.  The abstraction layer that replaces the regcache in this function
(register_readwrite) does not contain raw_update nor get_register_status.  I don't
think it would be right to add it, because it's quite specific to how the regcache
is implemented, so it would be a leaky abstraction.

>From what I understand from the comment above, the problem is that raw_write_part
does not return any feedback.  Would it be fine to make raw_write_part return
a value (probably a bool) to say whether it succeeded, and replace the current
error handling code with a check of that return value?  Something like:

bool success = regcache->raw_write_part (vr0 + reg_index, offset,
					 register_size (gdbarch, reg_nr), buffer);
if (!success)
  error (_("Cannot write to the checkpointed EFP register, "
	   "the corresponding vector register is unavailable."));

Simon

[1] https://sourceware.org/ml/gdb-patches/2018-10/msg00532.html