Hi Mike, On 24/06/25 10:03 am, Michael Meissner wrote: > On Fri, Jun 20, 2025 at 01:19:45PM -0500, Segher Boessenkool wrote: >> Hi! >> >> On Fri, Jun 20, 2025 at 10:38:30PM +0530, Surya Kumari Jangala wrote: >>> On 14/06/25 2:13 pm, Michael Meissner wrote: >>>> This is patch #4 of 4 to add -mcpu=future support to the PowerPC. >>> >>> I think this should be a separate patch in itself. As such, this >>> patch is not required to enable the -mcpu=future option. >> >> It can in theory be helpful to have it in the same series, but yeah, it >> certainly does not belong here. It should be a separate patch, and it >> should come with some evidence or at the very least some indication that >> it would be a good idea to have it at all, and proof that is not a *bad* >> idea! > > Sure, I can separate it, as there are other patches waiting for -mcpu=future > support to go in. The important thing is just having a switch to generate > these new instructions. > >>>> In the development for the power10 processor, GCC did not enable using the >>>> load >>>> vector pair and store vector pair instructions when optimizing things like >>> >>> s/things/functions >> >> "Things" is nicely non-specific, hehe. >> >>>> * config/rs6000/rs6000-cpus.def (ISA_FUTURE_MASKS_SERVER): Enable using >>> >>> Just FUTURE_MASKS_SERVER
Just to clarify, what I meant here is that the code does not have any macro named ISA_FUTURE_MASKS_SERVER. There is only FUTURE_MASKS_SERVER, and the commit message should specify this macro. Regards, Surya >> >> The existing masks are ISA_3_1_MASKS_SERVER (and many older ISAs before >> it), and POWER11_MASKS_SERVER . We do not have to call ISA 3.2 >> "Future", certainly not by IBM's lawyers, it isn't IBM who will publish >> Power Architecture revisions anyway! > > But it may be ISA_4_0 and not ISA_3_2 or maybe ISA_50. But we in GCC land > (and > those in LLVM land) have to have some name to use before the 'official' name > is > chosen. But as hardware gells, we need to get some support into GCC so that > it > ultimately goes in the Linux distributions. > > I have another set of patches that separates flag bits from ISA bits that I > have posted several times. But I'm trying to break the logjam to get patches > in, and I was proritizing getting -mcpu=future in first. > > Patches are like an agile system, in that generally people don't want a > sweeping set of changes. But instead they do a step-wise improvement. We > will > need at some point to add in changes that may or may not be in future PowerPC > architectures. > > Future is a convenient way to have these possible changes. We did as a > staging > method for power7, power8, power9, and power10 systems. I don't recall why we > deleted the -mcpu=future support, but these patches provide a way for future > patches to add possible experimental or future support. > > For example, when I worked at AMD, we had a massive set of changes that I did > and were put in. I was working with the hardware team providing compilers and > such. However, the company ultimately decided to go in a different direction, > and all of these 'future' patches were later removed. > >> Yeah, ISA_FUTURE makes no sense in the first place, "Future" here is a >> stand-in for the marketing name for the next IBM Power Server chip. The >> (lawyers') fear is that if we publish the expected name for the next >> generation server CPU, and also GCC support for that CPU, that then some >> potential customers can argue in the future (har har) that that was a >> promise. So we call it "Future", no specific version or timespan, and >> of course we cannot really predict the future, and future plans can >> always change, too. >> >> You can expect that in the future (when things have settled) we will >> just do a tree-wide search and replace. > > Sure, that happens. It is a simple mechanical process. But if/when that > machine comes out, we should leave in -mcpu=future to allow for development > beyond that machines. > >> >>>> * gcc/config/rs6000/rs6000.cc (rs6000_machine_from_flags): Disable >>>> -mblock-ops-vector-pair from influcing .machine selection. >>> >>> nit: "influencing" >> >> Speling fixes are never a nit! Attention to details is important. >> >>> Also, in rs6000.opt, mblock-ops-vector-pair is marked as Undocumented. >>> Should we >>> change this? >> >> Probably yes. If the option is worth being user-selectable at all, we >> should document it. > > -mblock-ops-vector-pair was a 'quick' hack to disable the memory/string > functions from generating vector pair load/store instructions. It was done > in > the late stages of power10 development, where there was one specific use case > that ran much slower, but it would be addressed in future releases. It > didn't > happen for power11, but it would be nice to have it ready if the -mcpu=future > changes make it into a future processor. > > I ran spec 2017, and on power10 hardware, one benchmark (526.blender_r) sped > up > by a small amount by using vector double load/store instructions instead of 2 > vector loads and 2 vector stores. >
