public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Sandiford <richard.sandiford@arm.com>
To: Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org>
Cc: HAO CHEN GUI <guihaoc@linux.ibm.com>,
	 Richard Biener <richard.guenther@gmail.com>,
	 Segher Boessenkool <segher@kernel.crashing.org>,
	 David <dje.gcc@gmail.com>,  "Kewen.Lin" <linkw@linux.ibm.com>,
	 Peter Bergner <bergner@linux.ibm.com>
Subject: Re: [PATCH] Add a new conversion for conditional ternary set into ifcvt [PR106536]
Date: Tue, 06 Dec 2022 09:36:23 +0000	[thread overview]
Message-ID: <mptiliodgbs.fsf@arm.com> (raw)
In-Reply-To: <CAFiYyc3m9RRrrVQR07e8eKZjcZ4Lq_6QEL79u86HHdApFJc1Fg@mail.gmail.com> (Richard Biener via Gcc-patches's message of "Thu, 24 Nov 2022 11:24:15 +0100")

Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> On Thu, Nov 24, 2022 at 8:25 AM HAO CHEN GUI <guihaoc@linux.ibm.com> wrote:
>>
>> Hi Richard,
>>
>>
>> 在 2022/11/24 4:06, Richard Biener 写道:
>> > Wouldn't we usually either add an optab or try to recog a canonical
>> > RTL form instead of adding a new target hook for things like this?
>>
>> Thanks so much for your comments. Please let me make it clear.
>>
>> Do you mean we should create an optab for "setb" pattern (the nested
>> if-then-else insn) and detect candidate insns in ifcvt pass? Then
>> generate the insn with the new optab?
>
> Yes, that would be one way to do it.  Another way would be to
> generate a (to be defined) canonical form of such instruction and
> see whether it can be recognized (whether there's a define_insn
> for it).
>
> Note that were just things that came into my mind here, I'm not too
> familiar with how we handle such situations but at least I'm not
> aware of dozens of target hooks to handle instruction availability.

Yeah, this was my reaction too.  The patch does use recog for the
conditional set itself, which is good, but I'm not sure from the patch
what the target hook is supposed to do in preparation for the recog.

I think it'd be better to avoid having too many hooks that take
noce_if_info parameters.  It's really just a bunch of internal
pass state, rather than something that was designed to be a public
interface.

Currently we have one hook that takes noce_if_info parameters,
for costing.  That's still one more than I'd like, but at least there
are no correctness concerns and, if someone changes noce_if_info
in future, just rebuilding cc1 for the affected targets should be
good enough as far as testing goes.  If we start using hooks for code
generation too, we're effectively distributing the ifcvt pass across
targets, which makes the pass harder to maintain.

Thanks,
Richard

  reply	other threads:[~2022-12-06  9:36 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-23  7:08 HAO CHEN GUI
2022-11-23 20:06 ` Richard Biener
2022-11-24  7:25   ` HAO CHEN GUI
2022-11-24 10:24     ` Richard Biener
2022-12-06  9:36       ` Richard Sandiford [this message]
2022-12-06 12:20       ` Segher Boessenkool
2022-12-02  2:49 ` Hans-Peter Nilsson
2022-12-02  6:04   ` HAO CHEN GUI

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=mptiliodgbs.fsf@arm.com \
    --to=richard.sandiford@arm.com \
    --cc=bergner@linux.ibm.com \
    --cc=dje.gcc@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=guihaoc@linux.ibm.com \
    --cc=linkw@linux.ibm.com \
    --cc=richard.guenther@gmail.com \
    --cc=segher@kernel.crashing.org \
    /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).