From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x42b.google.com (mail-wr1-x42b.google.com [IPv6:2a00:1450:4864:20::42b]) by sourceware.org (Postfix) with ESMTPS id 04203388C00B for ; Wed, 10 Jun 2020 10:56:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 04203388C00B Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wr1-x42b.google.com with SMTP id t18so1733304wru.6 for ; Wed, 10 Jun 2020 03:56:02 -0700 (PDT) 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; bh=d6r93oIADNBxyqMuq8dC03Lhm/JErSMGKmmOogDYBeo=; b=W+x2qfud8JEDmkIiPQ00rNmU9rtx0sz2gxwJGu2R9WLC5lHZXm8Kx7wwPm/+4vXQOk XYIM/AzALrSrc/77lBb1glm8qJ4QbYkMA7VIp0Kl8bcLJOPGeULZR+QE0zgM2l3DVAWd 3s59/qN1LpcJchANqYFMWt62LnExUVsQ0WcxDiiy+Zxh26xwRIjhbCs+pTxfvZe3O1Ta s55SFwhk9lSuxuyEdIIWqos6ucmEdFMlQKEzHLJdWlbYJm6u4H0730uRlBFuLYN5Th4I /6NzRoyPEoWPBDG4i5K9wY+UgsYI/IhVkoElpl8MMq0xXR1EzGQTkpea80GUwM+q4FyF 4kug== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=d6r93oIADNBxyqMuq8dC03Lhm/JErSMGKmmOogDYBeo=; b=hhEMEsCxMrrX3GQRNEGQpugAhV3hjydePSYieNlnu4/u/r5E/kJymo916o88SCKWpT VVZjwfD9cJuO5uVwF6e3pLpjx8uRcoehL6WRWRsW8kx7o3+q5cSAWM957vW/ZVk4QwMa 3pKXr9HvkOZrwqwPNrIiiwlv7rQ02qwCLzqmYcPPDTRxCvPu7r7LaaeVqRJ1UiLK/xP7 /b9lpb+gL4vb9oKLbePpxa/60jV1N2+gZQCe8ntB5Vi4Fztm5ur5wuyxCUvDuj2NAUJL 4wqviCdyYebaYzmAOlPTpFEY1sCdZoI3Rvh+gTkoT68fBrwolPOT2UeRMkVrCj4cCfnK 9D6w== X-Gm-Message-State: AOAM530nQSa2t/09dNU+d4/25TAphUAg/SzbGrsGQygVhNDd3oIgbf2I SRfzBHZ16F84zYLd5sq5KBId7A== X-Google-Smtp-Source: ABdhPJwZFsfp/yQRffPkaWjnaqmP96I8jAjI8OnkBBabkKGYWFJTvSOjJ1YkymqF1QsLBf8AjUIN4A== X-Received: by 2002:adf:e749:: with SMTP id c9mr3269366wrn.25.1591786561994; Wed, 10 Jun 2020 03:56:01 -0700 (PDT) Received: from localhost (host86-128-12-16.range86-128.btcentralplus.com. [86.128.12.16]) by smtp.gmail.com with ESMTPSA id d17sm8358870wrg.75.2020.06.10.03.56.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 10 Jun 2020 03:56:01 -0700 (PDT) Date: Wed, 10 Jun 2020 11:55:59 +0100 From: Andrew Burgess To: Nelson Chu Cc: Jim Wilson , Tom Tromey , gdb-patches@sourceware.org Subject: Re: [RFC] gdb/riscv: Improved register alias name creation Message-ID: <20200610105559.GI2737@embecosm.com> References: <1584007257-14466-1-git-send-email-nelson.chu@sifive.com> <87r1upefg8.fsf@tromey.com> <20200609173040.GE2737@embecosm.com> <20200609224723.GG2737@embecosm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Operating-System: Linux/5.6.15-200.fc31.x86_64 (x86_64) X-Uptime: 11:54:33 up 2 days, 1:01, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-Spam-Status: No, score=-3.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=no 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: Wed, 10 Jun 2020 10:56:04 -0000 * Nelson Chu [2020-06-10 17:31:00 +0800]: > Hi Andrew, > > The patch is pretty good to me. Thanks for your quick reply and fix. > I have some ideas and minor things when I try to support debug csr for > now. > > On Wed, Jun 10, 2020 at 6:47 AM Andrew Burgess > wrote: > > * Jim Wilson [2020-06-09 13:14:58 -0700]: > > > On Tue, Jun 9, 2020 at 10:30 AM Andrew Burgess > > > wrote: > > > > Looking then at the final PRIV_SPEC_CLASS_* field for each alias then > > > > we can see that currently we only want to take the alias from > > > > PRIV_SPEC_CLASS_1P11. For now then this is what I'm using to filter > > > > the aliases within GDB. > > > > > > This will do the right thing, but looks a little funny. It isn't > > > quite the right way to express what we want. I do think it is OK for > > > now, but we will have to be careful when maintaining binutils that we > > > don't break this assumption, or remember to update it when necessary. > > > > I agree. I certainly open to any other ideas. > > > > Without making changes to the DECLARE_CSR_ALIAS macro (and I don't > > know what changes I would make) I saw my options as either: > > > > - Ignore DECLARE_CSR_ALIAS, and hard code the "approved" aliases into > > GDB. Then it'll never break, we just need to remember to update > > the hard coded list when riscv-opc.h changes, or > > > > - Filter the alias list from riscv-opc.h. > > > > I went with the second option, partly because, if, from now on RISC-V > > doesn't reuse old CSR offsets for new CSRs, then any new aliases > > should be compatible.... I hope. > > I think we already have a consensus - It would be great if we only > support an alias with a new name, points at an existing CSR offset, > and the new name is a synonym for the existing CSR at that offset. > For the current FSF tree, there is only one CSR dscratch is an alias. > But I'm going to add the missing debug CSR to upstream, so there > should be more aliases in the future, and I think all of them follow > our consensus. > > DECLARE_CSR_ALIAS(dscratch, CSR_DSCRATCH0, CSR_CLASS_DEBUG, > PRIV_SPEC_CLASS_NONE, PRIV_SPEC_CLASS_NONE) > DECLARE_CSR_ALIAS(mcontrol, CSR_TDATA1, CSR_CLASS_DEBUG, > PRIV_SPEC_CLASS_NONE, PRIV_SPEC_CLASS_NONE) > DECLARE_CSR_ALIAS(icount, CSR_TDATA1, CSR_CLASS_DEBUG, > PRIV_SPEC_CLASS_NONE, PRIV_SPEC_CLASS_NONE) > DECLARE_CSR_ALIAS(itrigger, CSR_TDATA1, CSR_CLASS_DEBUG, > PRIV_SPEC_CLASS_NONE, PRIV_SPEC_CLASS_NONE) > DECLARE_CSR_ALIAS(etrigger, CSR_TDATA1, CSR_CLASS_DEBUG, > PRIV_SPEC_CLASS_NONE, PRIV_SPEC_CLASS_NONE) > DECLARE_CSR_ALIAS(textra32, CSR_TDATA3, CSR_CLASS_DEBUG, > PRIV_SPEC_CLASS_NONE, PRIV_SPEC_CLASS_NONE) > DECLARE_CSR_ALIAS(textra64, CSR_TDATA3, CSR_CLASS_DEBUG, > PRIV_SPEC_CLASS_NONE, PRIV_SPEC_CLASS_NONE) > > I think mcontrol, icount, itrigger and etrigger are the aliases to > tdata1. They all use 0x7a1 offset, and use type [top 5 bits] to > determine which one is used, so I believe they are similar to dscrach > and textra[32|64], and it's fine to define them by DECLARE_CSR_ALIAS. > The problem is that the debug csr and float csr actually belong to the > "unprivileged" CSR rather than privileged ones. That means, they > should be controlled by the debug specs and float specs rather than > the privileged specs. And in the future, vector CSR are also the > unprivileged ones controlled by the vector specs. But for now we > don't have a conclusion on how to let users choose the unprivileged > specs they want. Therefore, I plan to set their defined and aborted > versions to PRIV_SPEC_CLASS_NONE temporarily, including their aliases > (DECLARE_CSR_ALIAS). And the PRIV_SPEC_CLASS_NONE will be changed to > DEBUG_SPEC_CLASS_XXX and VECTOR_SPEC_CLASS_XXX in the future, > according to their CSR_CLASS_DEBUG and CSR_CLASS_V. This will affect > your current proposal, so I have an idea, > > How about Gdb creates the aliases just according to the > DECLARE_CSR_ALIAS, and don't need to care about the spec versions for > now. Binutils have to make sure that the CSR, which is defined by > DECLARE_CSR_ALIAS, must be the case like dscrach and itrigger > mentioned above. For the ubadaddr, sbadaddr and others, I assume we > will drop them in the future and don't need to worry about them. So I > prefer to use another macro rather than DECLARE_CSR_ALIAS to define > them. Maybe DECLARE_CSR_ALIAS_TEMP or DECLARE_CSR_ALIAS_1p10? Nelson, Thanks for your feedback. Do you plan to post a patch introducing the DECLARE_CSR_ALIAS_* macros soon(ish) or should I go ahead with the current version of my patch, and if/when you add the new macros you can update GDB to only use the one we care about, and drop the version check at that point? Thanks, Andrew