From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by sourceware.org (Postfix) with ESMTPS id 440723858002 for ; Tue, 30 Mar 2021 19:47:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 440723858002 Received: from pps.filterd (m0098396.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.43/8.16.0.43) with SMTP id 12UJXod3176232 for ; Tue, 30 Mar 2021 15:47:15 -0400 Received: from ppma03ams.nl.ibm.com (62.31.33a9.ip4.static.sl-reverse.com [169.51.49.98]) by mx0a-001b2d01.pphosted.com with ESMTP id 37jhrvk7f6-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 30 Mar 2021 15:47:15 -0400 Received: from pps.filterd (ppma03ams.nl.ibm.com [127.0.0.1]) by ppma03ams.nl.ibm.com (8.16.0.43/8.16.0.43) with SMTP id 12UJbVpA022664 for ; Tue, 30 Mar 2021 19:47:13 GMT Received: from b06cxnps3074.portsmouth.uk.ibm.com (d06relay09.portsmouth.uk.ibm.com [9.149.109.194]) by ppma03ams.nl.ibm.com with ESMTP id 37hvb8ayrv-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 30 Mar 2021 19:47:12 +0000 Received: from d06av21.portsmouth.uk.ibm.com (d06av21.portsmouth.uk.ibm.com [9.149.105.232]) by b06cxnps3074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 12UJlA3L30409020 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 30 Mar 2021 19:47:10 GMT Received: from d06av21.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 5B38052057; Tue, 30 Mar 2021 19:47:10 +0000 (GMT) Received: from oc3748833570.ibm.com (unknown [9.145.179.147]) by d06av21.portsmouth.uk.ibm.com (Postfix) with ESMTP id 51E1652051; Tue, 30 Mar 2021 19:47:10 +0000 (GMT) Received: by oc3748833570.ibm.com (Postfix, from userid 1000) id DC46ED803C6; Tue, 30 Mar 2021 21:47:09 +0200 (CEST) Date: Tue, 30 Mar 2021 21:47:09 +0200 From: Ulrich Weigand To: will schmidt Cc: gdb-patches@sourceware.org Subject: Re: [PATCH,rs6000,v3] gdb-power10-single-step Message-ID: <20210330194709.GB2343@oc3748833570.ibm.com> References: <2abf296edd936056d4e8932049fac751f0ae7152.camel@vnet.ibm.com> <896b57a450849aa1b45408b74f927d2ebd6833a3.camel@vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <896b57a450849aa1b45408b74f927d2ebd6833a3.camel@vnet.ibm.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-GCONF: 00 X-Proofpoint-GUID: 9gQtAecIZ23H6CFluSBxRc11nt-MZ2rn X-Proofpoint-ORIG-GUID: 9gQtAecIZ23H6CFluSBxRc11nt-MZ2rn X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.369, 18.0.761 definitions=2021-03-30_09:2021-03-30, 2021-03-30 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 mlxlogscore=999 mlxscore=0 adultscore=0 lowpriorityscore=0 bulkscore=0 impostorscore=0 spamscore=0 suspectscore=0 malwarescore=0 clxscore=1015 phishscore=0 priorityscore=1501 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2103250000 definitions=main-2103300141 X-Spam-Status: No, score=-2.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 30 Mar 2021 19:47:17 -0000 On Mon, Mar 29, 2021 at 05:43:53PM -0500, will schmidt wrote: > YYYY-MM-DD Will Schmidt > > gdb/ChangeLog: > * rs6000-tdep.c: Add support for single-stepping of > prefixed instructions. > > gdb/testsuite/ChangeLog: > * gdb.arch/powerpc-plxv-nonrel.s: Testcase using > non-relative plxv instructions. > * /gdb.arch/powerpc-plxv-nonrel.exp: Testcase harness. Stray '/' at the beginning. > + if ((ssize_t) len < PPC_INSN_SIZE) > + memory_error (TARGET_XFER_E_IO, from); Incorrect indentation. > + /* Check for PNOP and for prefixed instructions with R=0. Those > + instructions are safe to displace. Prefixed instructions with R=1 > + will read/write data to/from locations relative to the current PC. > + We would not be able to fixup after an instruction has written data > + into a displaced location, so decline to displace those instructions. */ > + if ((insn & OP_MASK) == 1 << 26) > + { > + if (((insn & PNOP_MASK) != PNOP_INSN) && > + ((insn & R_MASK) != R_ZERO)) GDB coding style is to have the && at the start of the second line. > + { > + displaced_debug_printf ("Not displacing prefixed instruction %08x at %s", > + insn, paddress (gdbarch, from)); Incorrect indentation. > + /* set offset to 8 if this is an 8-byte (prefixed) instruction. */ Start with a capital 'S' and end with two spaces after the '.'. > + if ((opcode) == 1 << 26) > + offset = 2 * PPC_INSN_SIZE; > + else > + offset = PPC_INSN_SIZE; Wrong indentation. > instructions. */ > for (insn_count = 0; insn_count < atomic_sequence_length; ++insn_count) > { > - loc += PPC_INSN_SIZE; > + int cur_insn = read_memory_integer (loc, PPC_INSN_SIZE, byte_order); > + if ((cur_insn & OP_MASK) == 1 << 26) > + loc += 2*PPC_INSN_SIZE; > + else > + loc += PPC_INSN_SIZE; > insn = read_memory_integer (loc, PPC_INSN_SIZE, byte_order); Hmm. This reads all instruction twice now, which we should avoid for performance reasons. But it is not actually necessary: at the start of this loop, "insn" is always the instruction at "loc", read either by the code before the loop or the previous iteration of the loop. So you can just use "insn" for the prefix check. (Also, indentation seems wrong :-)) Except for this one issue with reading the instructions twice, this looks now ready to commit (once the coding style issues are fixed). Thanks, Ulrich -- Dr. Ulrich Weigand GNU/Linux compilers and toolchain Ulrich.Weigand@de.ibm.com