On 10/26/22 11:06, Mayshao-oc wrote: > > Hi Martin: > Thanks for your patch, I comment the questions below.
Hi. :) > >> Hello. > >> I noticed this patch set which is kind of related to >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107364. > >> And I have a couple of questions: > >>1) I noticed you drop AVX and F16C features for the newly added "lujiazui". >>Why do you need it? >> I would expect these features would be properly detected by cpuid? > > Yes, these features could be detected by cpuid, and in respect of > functionality, these features are ok, but in respect of performance, these > features need further improvement, so we decide to drop it now, and add these > features back when performance meet our expectation. I see. So theoretically you can increase costs of the corresponding insns and that could be dropped now? But I'm not a costing expert. > >> 2) If you really need it, can you please test for me the attached patch? It >> should come up >> with a new function. > > I have tested the patch, It's ok. Good, I'm going to install it. > >> 3) Have question about: > >> else if (vendor == signature_CENTAUR_ebx && family < 0x07) >> cpu_model->__cpu_vendor = VENDOR_CENTAUR; >> else if (vendor == signature_SHANGHAI_ebx >> || vendor == signature_CENTAUR_ebx) > >> Are there any signature_CENTAUR_ebx models with family == 0x7 ? >> Similarly, are there any signature_SHANGHAI_ebx modes with family < 0x7 ? > > Yes, both cases exist in our products. Good. Then we miss a CPU features detection for (vendor == signature_CENTAUR_ebx && family < 0x07) aka https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107364. But it's not worth it as it's a legacy hardware, right? Cheers, Martin > >> Thanks, >> Martin > > BR > Mayshao