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

[Pedro Franco de Carvalho <pedromfc@linux.ibm.com>]

Hello Simon,

> From what I understand from the comment above, the problem is that raw_write_part
> does not return any feedback.

That's right, in the live frame an exception is raised only if the
target is called to try to write to the register, which doesn't happen
in regcache::raw_write_part when the initial read doesn't return a valid
status.  This initial read does go to the target, but it doesn't raise
an exception, it just marks the register as unavailable.

> [...]  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."));
>

This should work, although I wonder if it makes sense to return a
register_status like the private regcache::raw_write_part already does,
instead of a bool.

This should also work nicely if the write is done for a regular EFP
pseudo-register (as opposed to checkpointed one).  Before your patch
series, only the checkpointed version could result in an unavailable
read, but now I think this can also happen to the regular one for a
frame in which the underlying register wasn't saved.

In fact, couldn't this silent failure now also affect other arches that
use raw_write_part too, when the register is unavailable in a frame?

A possible solution might be to raise an exception directly from
register_readwriter::write_part when the read returns something other
than REG_VALID, although I'm not sure this would be correct in all
cases.

The only detail is that "checkpointed" should be removed from the error
message, so it becomes something like:

  if (regcache->get_register_status (vr0 + reg_index) != REG_VALID)
    error (_("Cannot write to the EFP register, the corresponding "
	     "vector register is unavailable."));

(I also added a space before the string continuation which was
previously missing).

Thanks!

--
Pedro Franco de Carvalho