public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jeff Law <jeffreyalaw@gmail.com>
To: Manolis Tsamis <manolis.tsamis@vrull.eu>,
	gcc-patches@gcc.gnu.org,
	Philipp Tomsich <philipp.tomsich@vrull.eu>,
	Robin Dapp <rdapp@linux.ibm.com>,
	Jakub Jelinek <jakub@redhat.com>,
	richard.sandiford@arm.com
Subject: Re: [PATCH v3 1/4] ifcvt: handle sequences that clobber flags in noce_convert_multiple_sets
Date: Fri, 10 Nov 2023 14:25:13 -0700	[thread overview]
Message-ID: <58423611-a24e-4c86-98c6-70b3b7dcda0e@gmail.com> (raw)
In-Reply-To: <mptedhqpfh2.fsf@arm.com>



On 10/19/23 13:41, Richard Sandiford wrote:
> Manolis Tsamis <manolis.tsamis@vrull.eu> writes:
>> This is an extension of what was done in PR106590.
>>
>> Currently if a sequence generated in noce_convert_multiple_sets clobbers the
>> condition rtx (cc_cmp or rev_cc_cmp) then only seq1 is used afterwards
>> (sequences that emit the comparison itself). Since this applies only from the
>> next iteration it assumes that the sequences generated (in particular seq2)
>> doesn't clobber the condition rtx itself before using it in the if_then_else,
>> which is only true in specific cases (currently only register/subregister moves
>> are allowed).
>>
>> This patch changes this so it also tests if seq2 clobbers cc_cmp/rev_cc_cmp in
>> the current iteration. This makes it possible to include arithmetic operations
>> in noce_convert_multiple_sets.
>>
>> gcc/ChangeLog:
>>
>> 	* ifcvt.cc (check_for_cc_cmp_clobbers): Use modified_in_p instead.
>> 	(noce_convert_multiple_sets_1): Don't use seq2 if it clobbers cc_cmp.
>>
>> Signed-off-by: Manolis Tsamis <manolis.tsamis@vrull.eu>
>> ---
>>
>> (no changes since v1)
>>
>>   gcc/ifcvt.cc | 49 +++++++++++++++++++------------------------------
>>   1 file changed, 19 insertions(+), 30 deletions(-)
> 
> Sorry for the slow review.  TBH I was hoping someone else would pick
> it up, since (a) I'm not very familiar with this code, and (b) I don't
> really agree with the way that the current code works.  I'm not sure the
> current dependency checking is safe, so I'm nervous about adding even
> more cases to it.  And it feels like the different ifcvt techniques are
> not now well distinguished, so that they're beginning to overlap and
> compete with one another.  None of that is your fault, of course. :)
I'd been hoping to get it it as well, particularly since I've got a TODO 
to sort out the conditional zero support in ifcvt.cc from various 
contibutors.  While there isn't any overlap I can see between that work 
and this submission from Manolis, it's close enough that if I'm going to 
get re-familiar with ifcvt.cc I figured I'd try to handle both.


jeff

  parent reply	other threads:[~2023-11-10 21:25 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-30 10:13 [PATCH v3 0/4] ifcvt: Allow if conversion of arithmetic in basic blocks with multiple sets Manolis Tsamis
2023-08-30 10:13 ` [PATCH v3 1/4] ifcvt: handle sequences that clobber flags in noce_convert_multiple_sets Manolis Tsamis
2023-10-19 19:41   ` Richard Sandiford
2023-10-20  7:04     ` Robin Dapp
2023-10-20  9:16       ` Richard Sandiford
2023-11-10 21:48         ` Jeff Law
2023-11-10 21:31       ` Jeff Law
2023-11-10 21:25     ` Jeff Law [this message]
2024-04-23 10:58     ` Manolis Tsamis
2023-08-30 10:13 ` [PATCH v3 2/4] ifcvt: Allow more operations in multiple set if conversion Manolis Tsamis
2023-10-19 19:46   ` Richard Sandiford
2023-11-10 21:53     ` Jeff Law
2023-11-13 12:47     ` Manolis Tsamis
2024-04-23 11:00     ` Manolis Tsamis
2023-08-30 10:13 ` [PATCH v3 3/4] ifcvt: Handle multiple rewired regs and refactor noce_convert_multiple_sets Manolis Tsamis
2023-11-10 23:20   ` Jeff Law
2023-11-13 12:40     ` Manolis Tsamis
2023-11-21 18:10       ` Manolis Tsamis
2023-08-30 10:14 ` [PATCH v3 4/4] ifcvt: Remove obsolete code for subreg handling in noce_convert_multiple_sets Manolis Tsamis
2023-11-10 22:03   ` Jeff Law
2023-11-13 12:43     ` Manolis Tsamis
2023-11-21 18:08       ` Manolis Tsamis
2023-09-18  8:18 ` [PATCH v3 0/4] ifcvt: Allow if conversion of arithmetic in basic blocks with multiple sets Manolis Tsamis
2023-10-19  6:53   ` Manolis Tsamis

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=58423611-a24e-4c86-98c6-70b3b7dcda0e@gmail.com \
    --to=jeffreyalaw@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=manolis.tsamis@vrull.eu \
    --cc=philipp.tomsich@vrull.eu \
    --cc=rdapp@linux.ibm.com \
    --cc=richard.sandiford@arm.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).