From: Jeff Law <jeffreyalaw@gmail.com>
To: Tsukasa OI <research_trasio@irq.a4lg.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH v3 1/1] RISC-V: Add support for 'XVentanaCondOps' reusing 'Zicond' support
Date: Tue, 5 Sep 2023 21:07:16 -0600 [thread overview]
Message-ID: <a24e5872-77a8-4315-b79c-ccc7617d75c0@gmail.com> (raw)
In-Reply-To: <32677516-2795-4e8f-a8d0-8304ba641c3a@irq.a4lg.com>
On 9/5/23 20:33, Tsukasa OI wrote:
>> Internally we have this as:
>>
>> (TARGET_ZICOND || TARGET_XVENTANACONDOPS)
>>
>> I don't really care, so I'm happy to go with yours.
>
> Because XVentanaCondOps instructions are only available on 64-bit target
> (I wanted to prevent misuses because we don't reject XVentanaCondOps on
> RV32), the target expression would be:
>
> (a) (TARGET_ZICOND || (TARGET_XVENTANACONDOPS && TARGET_64BIT))
>
> and I had three options to deal with it.
>
> 1. Use the plain expression (a)
> 2. Name the expression (a)
> 3. Enable TARGET_XVENTANACONDOPS only on 64-bit target
>
> I think option 2 is the simplest yet understandable.
Sure. It may also give us the option to roll in some of the thead code
at some point. Their conditional move support seems to line up pretty
well with zicond/xventanacondops too, though I haven't looked at it very
deeply yet.
>>
>
> I'm happy to hear that because I had no confidence so whether we can use
> #include to share common parts. I haven't tried yet but I believe we
> have to #include only common parts (not including dg instructions
> containing -march=..._zicond) so I will likely required to modify zicond
> tests as well.
You actually don't even have to break out the common parts. The dg-
directives in an included file aren't parsed by the dg framework.
>
> I'll submit PATCH v4 (not committing directly) as changes will be a bit
> larger (and Jeff's words seem "near approval" even after fixing the
> tests, not complete approval).
Sounds perfect. Given the bulk of the review work is already done, the
final review ack will be easy.
jeff
next prev parent reply other threads:[~2023-09-06 3:07 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-30 6:44 [RFC PATCH] " Tsukasa OI
2023-08-30 22:54 ` [RFC PATCH v2 0/1] " Tsukasa OI
2023-08-30 22:54 ` [RFC PATCH v2 1/1] " Tsukasa OI
2023-09-05 12:10 ` [PATCH v3 0/1] " Tsukasa OI
2023-09-05 12:10 ` [PATCH v3 1/1] " Tsukasa OI
2023-09-06 0:17 ` Jeff Law
2023-09-06 2:33 ` Tsukasa OI
2023-09-06 3:07 ` Jeff Law [this message]
2023-09-06 3:16 ` Palmer Dabbelt
2023-09-06 5:47 ` [PATCH v4 0/1] " Tsukasa OI
2023-09-06 5:47 ` [PATCH v4 1/1] " Tsukasa OI
2023-09-06 14:04 ` Jeff Law
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=a24e5872-77a8-4315-b79c-ccc7617d75c0@gmail.com \
--to=jeffreyalaw@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=research_trasio@irq.a4lg.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).