public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jeff Law <jeffreyalaw@gmail.com>
To: Palmer Dabbelt <palmer@rivosinc.com>
Cc: Kito Cheng <kito.cheng@gmail.com>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] RISC-V: Add the Zihpm and Zicntr extensions
Date: Tue, 22 Nov 2022 14:50:28 -0700	[thread overview]
Message-ID: <5bad6dad-f46e-620b-382c-31615b00d1bc@gmail.com> (raw)
In-Reply-To: <mhng-0ed782b3-d76e-46a4-b513-3db6f8b657d5@palmer-ri-x1c9>


On 11/22/22 08:29, Palmer Dabbelt wrote:
> On Tue, 22 Nov 2022 07:20:15 PST (-0800), jeffreyalaw@gmail.com wrote:
>>
>> On 11/20/22 18:36, Kito Cheng wrote:
>>>> So the idea here is just to define the extension so that it gets 
>>>> defined
>>>> in the ISA strings and passed through to the assembler, right?
>>> That will also define arch test marco:
>>>
>>> https://github.com/riscv-non-isa/riscv-c-api-doc/blob/master/riscv-c-api.md#architecture-extension-test-macro 
>>>
>>
>> Sorry I should have been clearer and included the test macro(s) as well.
>>
>> So a better summary would be that while it doesn't change the codegen
>> behavior in the compiler, it does provide the mechanisms to pass along
>> isa strings to other tools such as the assembler and signal via the test
>> macros that this extension is available.
>
> IMO the important bit here is that we're not adding any compatibility 
> flags, like we did when fence.i was removed from the ISA.  That's fine 
> as long as we never remove these instructions from the base ISA in the 
> software, but that's what's suggested by Andrew in the post.

Right.  IIUC these instructions were never supposed to be in the base 
ISA, but, in effect, snuck through.  We're retro-actively adding them as 
an extension, at least in terms of ISA strings & test macros.  We're 
currently (forever?) going to allow them in the assembler without 
strictly requiring the extension be on.


> It's a super weird one, but there's a bunch of cases in RISC-V where 
> we're told to just ignore words in the ISA manual.  Definitely a trap 
> for users (and we already had some Linux folks get bit by the counter 
> changes here), but that's just how RISC-V works.

Mistakes happen.  The key is to adjust for them as best as we can.    
I'd lean towards a stricter enforcement, bringing these 
instructions/extension in line with how we handle the others. It'd 
potentially mean source incompatibilities that would need to be fixed, 
but they shouldn't be difficult and we're still early enough in the game 
that we *could* take that route.  Andrew's position is more 
accommodating of existing code and while I may not 100% agree with his 
position, I understand it.


So while I'd lean towards a stricter checking, I can live with this 
approach.  I wouldn't mind hearing from Kito, Philipp and others though.


Jeff


  reply	other threads:[~2022-11-22 21:50 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-09  3:00 Palmer Dabbelt
2022-11-18  2:14 ` Christoph Müllner
2022-11-18  4:29   ` Palmer Dabbelt
2022-11-20 16:19 ` Jeff Law
2022-11-21  1:36   ` Kito Cheng
2022-11-22 15:20     ` Jeff Law
2022-11-22 15:29       ` Palmer Dabbelt
2022-11-22 21:50         ` Jeff Law [this message]
2022-11-22 22:00           ` Palmer Dabbelt
2024-01-19  8:01             ` Kito Cheng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5bad6dad-f46e-620b-382c-31615b00d1bc@gmail.com \
    --to=jeffreyalaw@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=kito.cheng@gmail.com \
    --cc=palmer@rivosinc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).