From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 126842 invoked by alias); 14 Nov 2018 14:29:50 -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 126824 invoked by uid 89); 14 Nov 2018 14:29:49 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-3.1 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=XML, kick, jhbFreeBSDorg, jhbfreebsdorg X-HELO: mail-wr1-f43.google.com Received: from mail-wr1-f43.google.com (HELO mail-wr1-f43.google.com) (209.85.221.43) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 14 Nov 2018 14:29:47 +0000 Received: by mail-wr1-f43.google.com with SMTP id l9so4635859wrt.13 for ; Wed, 14 Nov 2018 06:29:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=qlsyvwSqwxir7tvUng4OgPUs+5W34vQ3lJrXQ0UVnAI=; b=IA5BZV/Y3p6EsMtW+vtNoKmAaFWWQ6p7s1MQbm+xcNJTZvu3sE8N4sFeCztjtQQSbs udWkkgED7Zv957hpFdWUhVIO4bNDrjtWVd5etEyisdezQHYzxIewKuQuDn4B3lHKCITO hMZl2LCR3nYz6fYm/Q70M+xbKQ8by2T249NjnrlR87S3reUTmeVonpsWAsJgdNFIpXWr 52/HcAzP05v1QRjWJ2tuw7+I9nyCPnuKFxsTldUsSr8TM7Ik97JnwAueeAxF7efgo1qj 9UYzOpm1X7AkcgMM7Wim6Cypgi+TEWGTZF3INvFIzb8RpBC6y9Fy9Zcbbl+whnwybhdF HEpQ== Return-Path: Received: from localhost (host81-148-252-35.range81-148.btcentralplus.com. [81.148.252.35]) by smtp.gmail.com with ESMTPSA id p16-v6sm38862748wro.29.2018.11.14.06.29.43 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 14 Nov 2018 06:29:44 -0800 (PST) Date: Wed, 14 Nov 2018 14:29:00 -0000 From: Andrew Burgess To: John Baldwin Cc: gdb-patches@sourceware.org, jimw@sifive.com, palmer@sifive.com Subject: Re: [RFC] gdb/riscv: Add target description support Message-ID: <20181114142942.GL16539@embecosm.com> References: <20181108160745.24600-1-andrew.burgess@embecosm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Fortune: Hacking's just another word for nothing left to kludge. X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes X-SW-Source: 2018-11/txt/msg00223.txt.bz2 * John Baldwin [2018-11-08 10:32:57 -0800]: > On 11/8/18 8:07 AM, Andrew Burgess wrote: > > This commit adds target description support for riscv. > > > > I've used the split feature approach for specifying the architectural > > features, and the CSR feature is auto-generated from the riscv-opc.h > > header file. > > In general this looks fine to me (as far as I am familiar with the > target descriptions). The only possible question/comment I have is if > you considered describing fields of specific registers such as the FP > status registers or MSTATUS, etc. as fields in the XML to replace the > current special cases in riscv_print_one_register_info(). I took a look at switching over to using flag fields in the xml description, but in the end I decided against this. The main reason is that there appears to be a bug with registers described as a flag type, I was unable to assign to the register. What I saw was an invalid cast error. I defined the type like this: But then, when I try to assign to the register I see this error: (gdb) set $fflags=0x3 Invalid cast This same assignment works fine when the register is a straight integer. Next, I actually prefer the current 'info register $fflags' output, with all of the fields displayed all the time, so taking that away would feel like a step backward, though I could be convinced if everyone else feels strongly the other way. In the end I think that there's some issues that need to be resolved before we could move to using the type. I think this is something we should consider, but I'd like to kick this down the road for now. > I think the > XML can't handle enum values as riscv_print_one_register_info() uses for > some cases, but I think it would be able to handle many of the special > cases in that function. I agree, currently we're not going to be able to completely kill the info registers override, and as a result I'm not that fussed about having extra cases in here. > > Some related-ish questions (though not about this patch): I wonder if we > can do things with pseudo registers to automatically derive FFLAGS and > FRM if a target provides FCSR. I like this idea a lot, and I definitely plan to implement this, just because I think it would be a neat feature. However, my understanding is that FFLAGS and FRM _are_ real CSRs, at least in the sense that there's a real CSR offset from which we can read to extract the FFLAGS or FRM part of FCSR. So, initially I'd prefer to merge this with these registers as real registers. But I think it would be great if for targets that only announce FCSR, we could automatically emulate FRM and FCSR. Thank you for your review. I plan to post a revised patch soon. Thanks, Andrew