From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 80574 invoked by alias); 15 Feb 2017 12:01:48 -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 80549 invoked by uid 89); 15 Feb 2017 12:01:48 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=1.2 required=5.0 tests=AWL,BAYES_50,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 spammy=inspired, facing, sk:PPC_FEA, sk:ppc_fea X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0b-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.158.5) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 15 Feb 2017 12:01:38 +0000 Received: from pps.filterd (m0098414.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.20/8.16.0.20) with SMTP id v1FBwrJA016990 for ; Wed, 15 Feb 2017 07:01:36 -0500 Received: from e24smtp04.br.ibm.com (e24smtp04.br.ibm.com [32.104.18.25]) by mx0b-001b2d01.pphosted.com with ESMTP id 28mp5a92eu-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Wed, 15 Feb 2017 07:01:36 -0500 Received: from localhost by e24smtp04.br.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 15 Feb 2017 10:01:34 -0200 Received: from d24dlp01.br.ibm.com (9.18.248.204) by e24smtp04.br.ibm.com (10.172.0.140) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Wed, 15 Feb 2017 10:01:31 -0200 Received: from d24relay02.br.ibm.com (d24relay02.br.ibm.com [9.18.232.42]) by d24dlp01.br.ibm.com (Postfix) with ESMTP id 02FB9352005F for ; Wed, 15 Feb 2017 07:00:58 -0500 (EST) Received: from d24av02.br.ibm.com (d24av02.br.ibm.com [9.8.31.93]) by d24relay02.br.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id v1FC1N9J28049532 for ; Wed, 15 Feb 2017 10:01:31 -0200 Received: from d24av02.br.ibm.com (localhost [127.0.0.1]) by d24av02.br.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id v1FC0Oer019554 for ; Wed, 15 Feb 2017 10:00:25 -0200 Received: from [9.85.148.239] ([9.85.148.239]) by d24av02.br.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id v1FC0MD2018835; Wed, 15 Feb 2017 10:00:23 -0200 Subject: Re: [PATCH v2] [ppc64] Add POWER8 atomic sequences single-stepping support To: Luis Machado , gdb-patches@sourceware.org References: <1486350182-13993-1-git-send-email-emachado@linux.vnet.ibm.com> <38fe05a1-770c-e9f1-ecc0-042dfe0a470e@codesourcery.com> <7fe00f19-6922-fd6c-fa97-a779ffebac25@linux.vnet.ibm.com> Cc: uweigand@de.ibm.com From: Edjunior Barbosa Machado Date: Wed, 15 Feb 2017 12:01:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 17021512-0028-0000-0000-00000195A47A X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17021512-0029-0000-0000-00001492A62B Message-Id: <98042cab-d856-6426-e9a6-f7256ed789d0@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-02-15_06:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=2 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1612050000 definitions=main-1702150117 X-IsSubscribed: yes X-SW-Source: 2017-02/txt/msg00407.txt.bz2 Hi Luis, thanks for the review once again. Just few doubts below. On 02/15/2017 08:00 AM, Luis Machado wrote: > On 02/13/2017 06:47 PM, Edjunior Barbosa Machado wrote: >> diff --git a/gdb/testsuite/gdb.arch/power8-atomic-inst.S >> b/gdb/testsuite/gdb.arch/power8-atomic-inst.S >> new file mode 100644 >> index 0000000..daa3337 >> --- /dev/null >> +++ b/gdb/testsuite/gdb.arch/power8-atomic-inst.S > > I don't know if there are other powerpc initiatives out there other than > IBM's power 8/9 that are using these instructions. If there are, > renaming power8 to something generic would be best. Otherwise i don't > see a problem with leaving this and fixing it in the future if some > other manufacturer shows up using ISA 2.06/2.07. > > I thought i'd mention it though. I'm also not aware of other initiatives that implement these instructions. This name was more inspired on others testcases from gas focused on these POWER8/ISA 2.07 instructions like gas/testsuite/gas/ppc/power8.*. Any suggestion about what would be a better name here? > >> diff --git a/gdb/testsuite/gdb.arch/power8-atomic-inst.c >> b/gdb/testsuite/gdb.arch/power8-atomic-inst.c >> new file mode 100644 >> index 0000000..535e057 >> --- /dev/null >> +++ b/gdb/testsuite/gdb.arch/power8-atomic-inst.c > > Same as above about mentioning power8 in the filename. > >> @@ -0,0 +1,42 @@ >> +/* Copyright 2017 Free Software Foundation, Inc. >> + >> + This file is part of GDB. >> + >> + This program is free software; you can redistribute it and/or modify >> + it under the terms of the GNU General Public License as published by >> + the Free Software Foundation; either version 3 of the License, or >> + (at your option) any later version. >> + >> + This program is distributed in the hope that it will be useful, >> + but WITHOUT ANY WARRANTY; without even the implied warranty of >> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + GNU General Public License for more details. >> + >> + You should have received a copy of the GNU General Public License >> + along with this program. If not, see >> . */ >> + >> +#include >> + >> +typedef Elf64_auxv_t auxv_t; >> + >> +#ifndef PPC_FEATURE2_ARCH_2_07 >> +#define PPC_FEATURE2_ARCH_2_07 0x80000000 >> +#endif >> + >> +extern void test_atomic_sequences (void); >> + >> +int >> +main (int argc, char *argv[], char *envp[], auxv_t auxv[]) >> +{ >> + int i; >> + >> + for (i = 0; auxv[i].a_type != AT_NULL; i++) >> + if (auxv[i].a_type == AT_HWCAP2) { >> + if (!(auxv[i].a_un.a_val & PPC_FEATURE2_ARCH_2_07)) >> + return 1; >> + break; >> + } >> + >> + test_atomic_sequences (); >> + return 0; >> +} > > Since we've separated testing of these new instructions from the older > ones, dropped the power8 compiler switch and are not expecting SIGILL > anymore, do we still need a runtime check here? > > Checking the auxv is also Linux-specific and won't work for bare-metal. > > I think letting the test give a compilation error if the compiler > doesn't support the instructions is fine and also an indication the test > shouldn't run. > > If the compiler does support generating such instructions and the target > itself doesn't support them, we will have a problem. But it would be up > to whoever is building the program to pass the correct switches to the > compiler. In any case, this can be handled in the future if this > situation arises, right? > Actually this is a problem I'm already facing when testing more recent compilers on POWER7 machines for example. It builds OK but fails with SIGILL when running (that's why I initially tried expecting for SIGILL), then switched to this runtime check. Do you have any suggestion about what would be the best strategy that would work for ppc64 bare-metal too? Thanks, -- Edjunior