public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
From: "juzhe.zhong@rivai.ai" <juzhe.zhong@rivai.ai>
To: tamar.christina <Tamar.Christina@arm.com>
Cc: "Richard Biener" <richard.guenther@gmail.com>,
	 "Robin Dapp" <rdapp.gcc@gmail.com>,  gcc <gcc@gcc.gnu.org>
Subject: Re: Re: Loop vectorizer optimization questions
Date: Tue, 9 Jan 2024 17:21:46 +0800	[thread overview]
Message-ID: <5D947A55A07C6293+202401091721458721862@rivai.ai> (raw)
In-Reply-To: <ZZ0LDoo9oUIOvq21@arm.com>

[-- Attachment #1: Type: text/plain, Size: 4186 bytes --]

I see. Thanks Tamar.

I am willing to to investigate Arm's initial patch to see what else we need in that patch.

Since min/max reduction with index can improve SPEC performance, I will take a look at it in GCC-15.

Thanks a lot !



juzhe.zhong@rivai.ai
 
From: Tamar Christina
Date: 2024-01-09 16:59
To: 钟居哲
CC: richard.guenther; rdapp.gcc; gcc
Subject: Re: RE: Loop vectorizer optimization questions
Hi,
 
The 01/08/2024 22:46, 钟居哲 wrote:
> Oh. It's nice to see you have support min/max index reduction.
> 
> I knew your patch can handle this following:
> 
> 
> int idx = ii;
> int max = mm;
> for (int i = 0; i < n; ++i) {
>   int x = a[i];
>   if (max < x) {
>     max = x;
>     idx = i;
>   }
> }
> 
> But I wonder whether your patch can handle this:
> 
> int idx = ii;
> int max = mm;
> for (int i = 0; i < n; ++i) {
>   int x = a[i];
>   if (max <= x) {
>     max = x;
>     idx = i;
>   }
> }
> 
 
The last version of the patch we sent handled all conditionals:
 
https://inbox.sourceware.org/gcc-patches/DB9PR08MB6603DCCB35007D83C6736167F5599@DB9PR08MB6603.eurprd08.prod.outlook.com/
 
There are some additional testcases in the patch for all these as well.
 
> Will you continue to work on min/max with index ?
 
I don't know if I'll have the free time to do so, that's the reason I haven't resent the new one.
The engineer who started it no longer works for Arm.
 
> Or you want me to continue this work base on your patch ?
> 
> I have an initial patch which roughly implemented LLVM's approach but turns out Richi doesn't want me to apply LLVM's approach so your patch may be more reasonable than LLVM's approach.
> 
 
When Richi reviewed it he wasn't against the approach in the patch https://inbox.sourceware.org/gcc-patches/nycvar.YFH.7.76.2105071320170.9200@zhemvz.fhfr.qr/
but he wanted the concept of a dependent reduction to be handle more generically, so we could extend it in the future.
 
I think, from looking at Richi's feedback is that he wants vect_recog_minmax_index_pattern to be more general. We've basically hardcoded the reduction type,
but it could just be a property on STMT_VINFO.
 
Unless I'm mistaken the patch already relies on first finding both reductions, but we immediately try to resolve the relationship using vect_recog_minmax_index_pattern.
Instead I think what Richi wanted was for us to keep track of reductions that operate on the same induction variable and after we finish analysing all reductions we
try to see if any reductions we kept track of can be combined.
 
Basically just separate out the discovery and tieing of the reductions.
 
Am I right here Richi?
 
I think the codegen part can mostly be used as is, though we might be able to do better for VLA.
 
So it should be fairly straight forward to go from that final patch to what Richi wants, but.. I just lack time.
 
If you want to tackle it that would be great :)
 
Thanks,
Tamar
 
> Thanks.
> ________________________________
> juzhe.zhong@rivai.ai
> 
> From: Tamar Christina<mailto:Tamar.Christina@arm.com>
> Date: 2024-01-09 01:50
> To: 钟居哲<mailto:juzhe.zhong@rivai.ai>; gcc<mailto:gcc@gcc.gnu.org>
> CC: rdapp.gcc<mailto:rdapp.gcc@gmail.com>; richard.guenther<mailto:richard.guenther@gmail.com>
> Subject: RE: Loop vectorizer optimization questions
> >
> > Also, another question is that I am working on min/max reduction with index, I
> > believe it should be in GCC-15, but I wonder
> > whether I can pre-post for review in stage 4, or I should post patch (min/max
> > reduction with index) when GCC-15 is open.
> >
> 
> FWIW, We tried to implement this 5 years ago https://gcc.gnu.org/pipermail/gcc-patches/2019-November/534518.html
> and you'll likely get the same feedback if you aren't already doing so.
> 
> I think Richard would prefer to have a general framework these kinds of operations.  We never got around to doing so
> and it's still on my list but if you're taking care of it 
> 
> Just though I'd point out the previous feedback.
> 
> Cheers,
> Tamar
> 
> > Thanks.
> >
> >
> > juzhe.zhong@rivai.ai
 
-- 
 

  reply	other threads:[~2024-01-09  9:22 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-08 13:57 钟居哲
2024-01-08 17:50 ` Tamar Christina
2024-01-08 22:46   ` 钟居哲
2024-01-09  8:59     ` Tamar Christina
2024-01-09  9:21       ` juzhe.zhong [this message]
2024-01-09 13:10 ` Richard Biener

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=5D947A55A07C6293+202401091721458721862@rivai.ai \
    --to=juzhe.zhong@rivai.ai \
    --cc=Tamar.Christina@arm.com \
    --cc=gcc@gcc.gnu.org \
    --cc=rdapp.gcc@gmail.com \
    --cc=richard.guenther@gmail.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).