From patchwork Mon Jun 4 11:19:35 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Alan Hayward X-Patchwork-Id: 27611 Received: (qmail 27403 invoked by alias); 4 Jun 2018 11:19:43 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 27393 invoked by uid 89); 4 Jun 2018 11:19:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.0 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, MIME_BASE64_BLANKS, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 spammy=Program, mandatory, array_size, tdesc_data X-HELO: EUR01-HE1-obe.outbound.protection.outlook.com Received: from mail-he1eur01on0055.outbound.protection.outlook.com (HELO EUR01-HE1-obe.outbound.protection.outlook.com) (104.47.0.55) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 04 Jun 2018 11:19:40 +0000 Received: from DB6PR0802MB2133.eurprd08.prod.outlook.com (10.172.226.148) by DB6PR0802MB2311.eurprd08.prod.outlook.com (10.172.228.15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.820.11; Mon, 4 Jun 2018 11:19:36 +0000 Received: from DB6PR0802MB2133.eurprd08.prod.outlook.com ([fe80::d984:bdee:1856:c64]) by DB6PR0802MB2133.eurprd08.prod.outlook.com ([fe80::d984:bdee:1856:c64%7]) with mapi id 15.20.0820.010; Mon, 4 Jun 2018 11:19:36 +0000 From: Alan Hayward To: Simon Marchi , Pedro Alves CC: "gdb-patches@sourceware.org" , nd Subject: Re: [PATCH 4/8] Enable SVE for GDB Date: Mon, 4 Jun 2018 11:19:35 +0000 Message-ID: <21329FE3-8072-44B4-A988-D2A7A15A0E1C@arm.com> References: <20180511105256.27388-1-alan.hayward@arm.com> <20180511105256.27388-5-alan.hayward@arm.com> <3641fc23-e712-88f9-327e-bc795b3a1255@ericsson.com> In-Reply-To: <3641fc23-e712-88f9-327e-bc795b3a1255@ericsson.com> authentication-results: spf=none (sender IP is ) smtp.mailfrom=Alan.Hayward@arm.com; x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; DB6PR0802MB2311; 7:mudVASQOhz1L63boIX/ybFGTnjuEVrQtcJtZ/Tzxxi7TpA3bWIAyPKU3NUhcZH22pIuLNAVRD52avdFNjHmjAQPQoENzS4iz5k0R4jajldN1Qr/cXELvBkLs6WVzs9gU9gM0MEizafAZwKw+/YGwY7yNGI0Vz+I/p0hBZFMX0ulJelVW1E4fKJKJb+zms/WEffwI0yTVY6ONKyl5TnEeVSC/Z4FLfnSxzYejvMALhMymcwIuluT9Pzfvjj08+8J7 x-ms-exchange-antispam-srfa-diagnostics: SOS; x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(7020095)(4652020)(48565401081)(5600026)(4534165)(4627221)(201703031133081)(201702281549075)(2017052603328)(7153060)(7193020); SRVR:DB6PR0802MB2311; x-ms-traffictypediagnostic: DB6PR0802MB2311: nodisclaimer: True x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(37575265505322); x-ms-exchange-senderadcheck: 1 x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(8121501046)(5005006)(10201501046)(3002001)(93006095)(93001095)(3231254)(944501410)(52105095)(6055026)(149027)(150027)(6041310)(20161123558120)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123562045)(20161123560045)(20161123564045)(6072148)(201708071742011)(7699016); SRVR:DB6PR0802MB2311; BCL:0; PCL:0; RULEID:; SRVR:DB6PR0802MB2311; x-forefront-prvs: 069373DFB6 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(376002)(39860400002)(346002)(39380400002)(396003)(366004)(189003)(199004)(53546011)(54906003)(110136005)(305945005)(476003)(76176011)(2616005)(316002)(478600001)(83716003)(81166006)(6512007)(8936002)(57306001)(26005)(106356001)(7736002)(99286004)(102836004)(11346002)(446003)(3846002)(6116002)(59450400001)(486006)(186003)(575784001)(105586002)(6506007)(36756003)(3660700001)(3280700002)(72206003)(25786009)(53936002)(33656002)(5250100002)(2900100001)(86362001)(6436002)(6486002)(4326008)(229853002)(5660300001)(50226002)(66066001)(14454004)(82746002)(81156014)(68736007)(8676002)(6246003)(97736004)(2906002); DIR:OUT; SFP:1101; SCL:1; SRVR:DB6PR0802MB2311; H:DB6PR0802MB2133.eurprd08.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: 8sWHFL6SexGDe6+tmtN0JP/VrwYEwWDRn7Zz+pXOxlxrDFcXnWvEeDY1XBivraujNYK+YojNGlebEBKc6857wlWKTc8VrHTJQ3Hwkt+fTOqPNR3A0ckIZ6lDn0g7Npsq/zdUn5PHubbW7B59m8UmyU3yxQrJI+uRx0G0kpXK74PdBC5yz5sY7jRt5ELodwF0 spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-ID: <5E0292D4B638D640A59B445041E3DC95@eurprd08.prod.outlook.com> MIME-Version: 1.0 X-MS-Office365-Filtering-Correlation-Id: 759a6f21-5e1b-478c-c487-08d5ca0d0e04 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-Network-Message-Id: 759a6f21-5e1b-478c-c487-08d5ca0d0e04 X-MS-Exchange-CrossTenant-originalarrivaltime: 04 Jun 2018 11:19:36.0201 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6PR0802MB2311 X-IsSubscribed: yes Push with minor changes as suggested below. > On 31 May 2018, at 12:55, Simon Marchi wrote: > > On 2018-05-11 06:52 AM, Alan Hayward wrote: >> This patch enables SVE support for GDB by reading the VQ when >> creating a target description. >> >> It also ensures that SVE is taken into account when creating >> the tdep structure, and stores the current VQ value directly >> in tdep. >> >> With this patch, gdb on an aarch64 system with SVE will now detect >> SVE. The SVE registers will be displayed (but the contents will be >> invalid). The following patches fill out the contents. > > LGTM, with some nits. > >> @@ -2911,25 +2933,45 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) >> /* Validate the descriptor provides the mandatory core R registers >> and allocate their numbers. */ >> for (i = 0; i < ARRAY_SIZE (aarch64_r_register_names); i++) >> - valid_p &= >> - tdesc_numbered_register (feature, tdesc_data, AARCH64_X0_REGNUM + i, >> - aarch64_r_register_names[i]); >> + valid_p &= tdesc_numbered_register (feature_core, tdesc_data, >> + AARCH64_X0_REGNUM + i, >> + aarch64_r_register_names[i]); >> >> num_regs = AARCH64_X0_REGNUM + i; >> >> - /* Look for the V registers. */ >> - feature = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.fpu"); >> - if (feature) >> + /* Add the V registers. */ >> + if (feature_fpu != NULL) >> { >> + gdb_assert (feature_sve == NULL); > > Again, if this situation can result from a bad input passed to GDB (a bad tdesc > sent by the remote), we shouldn't gdb_assert on it, but show an error message > and gracefully fail. Ok. Updated to use _error. if (feature_sve != NULL) error (_("Program contains both fpu and SVE features.")); > >> + >> /* Validate the descriptor provides the mandatory V registers >> - and allocate their numbers. */ >> + and allocate their numbers. */ >> for (i = 0; i < ARRAY_SIZE (aarch64_v_register_names); i++) >> - valid_p &= >> - tdesc_numbered_register (feature, tdesc_data, AARCH64_V0_REGNUM + i, >> - aarch64_v_register_names[i]); >> + valid_p &= tdesc_numbered_register (feature_fpu, tdesc_data, >> + AARCH64_V0_REGNUM + i, >> + aarch64_v_register_names[i]); >> >> num_regs = AARCH64_V0_REGNUM + i; >> + } >> + >> + /* Add the SVE registers. */ >> + if (feature_sve != NULL) >> + { >> + gdb_assert (feature_fpu == NULL); > > Same here. I removed this one as it’s redundant - it’ll be caught by the error above. > >> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h >> index c9fd7b3578..046de6228f 100644 >> --- a/gdb/aarch64-tdep.h >> +++ b/gdb/aarch64-tdep.h >> @@ -73,6 +73,15 @@ struct gdbarch_tdep >> >> /* syscall record. */ >> int (*aarch64_syscall_record) (struct regcache *regcache, unsigned long svc_number); >> + >> + /* The VQ value for SVE targets, or zero if SVE is not supported. */ >> + long vq; >> + >> + /* Returns true if the target supports SVE. */ >> + bool has_sve () >> + { >> + return vq != 0; >> + } > > This method can be const. > Done. > On 31 May 2018, at 15:59, Pedro Alves wrote: > > On 05/11/2018 11:52 AM, Alan Hayward wrote: >> --- a/gdb/aarch64-tdep.c >> +++ b/gdb/aarch64-tdep.c >> @@ -2873,6 +2873,26 @@ aarch64_read_description (long vq) >> return tdesc; >> } >> >> +/* Return the VQ used when creating the target description TDESC. */ >> + >> +static long >> +aarch64_get_tdesc_vq (const struct target_desc *tdesc) > > Is this use of "long" significant? I mean, is it assuming 64-bit? > I ask because longs are not 64-bit on x64 Windows, so it would > do the wrong thing when cross debugging. > Fixed by using both follow on patch "[PATCH] Use uint64_t for SVE VQ” and obvious patch below: Thanks! Alan. diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index 6674b7654e..0172e4ccd1 100644 --- a/gdb/aarch64-tdep.c +++ b/gdb/aarch64-tdep.c @@ -2875,7 +2875,7 @@ aarch64_read_description (uint64_t vq) /* Return the VQ used when creating the target description TDESC. */ -static long +static uint64_t aarch64_get_tdesc_vq (const struct target_desc *tdesc) { const struct tdesc_feature *feature_sve; @@ -2888,7 +2888,8 @@ aarch64_get_tdesc_vq (const struct target_desc *tdesc) if (feature_sve == nullptr) return 0; - long vl = tdesc_register_size (feature_sve, aarch64_sve_register_names[0]); + uint64_t vl = tdesc_register_size (feature_sve, + aarch64_sve_register_names[0]); return sve_vq_from_vl (vl); } diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h index b6b9b30e71..598a0aafa2 100644 --- a/gdb/aarch64-tdep.h +++ b/gdb/aarch64-tdep.h @@ -75,7 +75,7 @@ struct gdbarch_tdep int (*aarch64_syscall_record) (struct regcache *regcache, unsigned long svc_number); /* The VQ value for SVE targets, or zero if SVE is not supported. */ - long vq; + uint64_t vq; /* Returns true if the target supports SVE. */ bool has_sve () const