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/db9pr08mb6603dccb35007d83c6736167f5...@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.9...@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.zh...@rivai.ai
>
> From: Tamar Christina<mailto:tamar.christ...@arm.com>
> Date: 2024-01-09 01:50
> To: 钟居哲<mailto:juzhe.zh...@rivai.ai>; gcc<mailto:gcc@gcc.gnu.org>
> CC: rdapp.gcc<mailto:rdapp....@gmail.com>;
> richard.guenther<mailto:richard.guent...@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.zh...@rivai.ai
--
