From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 98350 invoked by alias); 9 Feb 2019 05:51:33 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 98330 invoked by uid 89); 9 Feb 2019 05:51:31 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-4.9 required=5.0 tests=BAYES_00,GIT_PATCH_1,SPF_HELO_PASS,SPF_PASS,UNSUBSCRIBE_BODY autolearn=ham version=3.3.2 spammy=H*Ad:D*ca, *buffer X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 09 Feb 2019 05:51:30 +0000 Received: from [10.0.0.178] (unknown [192.222.164.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 78EE81E077; Sat, 9 Feb 2019 00:51:25 -0500 (EST) Subject: Re: [PATCH v5 12/12] [PowerPC] Add support for HTM registers To: Pedro Franco de Carvalho , gdb-patches@sourceware.org Cc: uweigand@de.ibm.com, edjunior@gmail.com References: <20181022223242.7858-1-pedromfc@linux.ibm.com> <20181022223242.7858-13-pedromfc@linux.ibm.com> From: Simon Marchi Message-ID: Date: Sat, 09 Feb 2019 05:51:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.0 MIME-Version: 1.0 In-Reply-To: <20181022223242.7858-13-pedromfc@linux.ibm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2019-02/txt/msg00104.txt.bz2 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