> -----Original Message----- > From: Segher Boessenkool <[email protected]> > Sent: 12 October 2025 16:58 > To: Tamar Christina <[email protected]> > Cc: [email protected]; nd <[email protected]>; [email protected]; > [email protected]; [email protected] > Subject: Re: [PATCH 6/9][rs6000]: convert widen_[us]sum into convert optab > [PR122069] > > Hi! > > On Fri, Oct 03, 2025 at 10:46:34AM +0100, Tamar Christina wrote: > > This patch is a mechanical rewrite of the widen_[us]sum optabs from a direct > to > > a conversion optab. > > There is no optab (or table of any other kind) in this patch. Instead, > it changes some pattern names, from well-known sort-of-standard names to > not-so-standard names. > > Could you please make these new names standard (i.e. DOCUMENTED) first? > And then refer to that documentation in this patch :-)
They are documented, they are changed in patch 1. [1] [1] https://patchwork.ozlabs.org/project/gcc/patch/[email protected]/ This patch is blocking the documentation update commit :) > > > The result of which requires the output mode to be added to > > the existing patterns. > > That sounds good :-) > > > * config/rs6000/altivec.md (widen_usum<mode>3): Renamed ... > > Please never use passive tense in changelogs. It should read like "do > this, and do that", not like "something or other might be done (behind > the scenes)". > > > (widen_usumv4si<mode>3): ... into this. > > Those new names are incorrect in any case, it should be > widen_usumv4si<mode>2! There are only *two* <mode> thing operands in > the pattern, after all. That is what the number means: something like > "addsi3" is sort of shorthand for "addsisisi" (and addsi2di means > addsisidi). > The number just indicates the number of operands, there are 3, so the number is correct, that's the convention established by the middle-end and that's what the patch follows. Before this there was only 1 mode and it was still 3. > So, (with the faulty names) this patch results in some better generated > code? You might want to show that in the patch message, to motivate > this change a bit :-) The patch allows you to create additional conversions should the backend want to. However it does not change the codegen of the backend. The patch simply allows a target to implement multiple conversions from the same Source type. So you can now implement QI -> SI and QI -> HI and the vectorizer will choose one based on costing. The change is mechanical, and done over all targets that have implemented this optab to keep them working. I'll send a new changelog with present tense. Thanks, Tamar > > Thanks, > > > Segher
