Hi! On Sun, Oct 12, 2025 at 05:00:59PM +0000, Tamar Christina wrote: > > -----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] > > > > 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 :)
You have all sequencing reversed, then. Docs come first, always. > > > (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. Some people use that convention. And some do not. That latter convention has older history (and makes way more sense). How can your proposed naming distinguish between an add with a SIMode and two DImode operands, and one with two SImode and onbe DImode ops? It cannot. > > 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. In what way? This is just confusing me more. > The change is mechanical, and done over all targets that have implemented > this optab to keep them working. "Optab". You keep using that word. What does it mean? We are talking about a named RTL pattern here, AFAICS. I'll look at the patch you sent the URL to now. Segher
