Neoverse V2: remove `sm3`, `sm4` and `svesm4`

paolotricerri commented 3 months ago

fixes #102

paolotricerri commented 3 months ago

@dslarm

giordano commented 3 months ago

Can you please edit the title of the PR to be self descriptive? Just referencing an issue number isn't useful.

alalazo commented 3 months ago

Can you please edit the title of the PR to be self descriptive? Just referencing an issue number isn't useful.

Asked the same thing in #105 :sweat_smile: A brief description of the PR would be welcome too.

dslarm commented 3 months ago

I can confirm that this patch fixes my initial issue: the sm2/3 and svesm4 are not present on the V2 system that I am using. with this patch,

poetry run python
Python 3.10.12 (main, Nov 20 2023, 15:14:05) [GCC 11.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import archspec.cpu
>>> print (archspec.cpu.host())
neoverse_v2
>>>

and neoverse_n1 without it.

alalazo commented 3 months ago

Can you add a sample of the misidentified cpu to the tests? This would allow us to unit test this failed detection in archspec

paolotricerri commented 2 months ago

@alalazo , just as a test, so will have to confirm with @dslarm properly, I have taken the archspec repo, removed the SM crypto flags from neoverse_v2 as well as from the linux-rhel9-neoverse_v2 test file and the function cpu.detect.host() returns a microarchitecture for neoverse_v2 because of the sorting function (and so the length of ancestors and features) and not neoverse_n1.

I'll confirm @dslarm 's test and try to add a similar test to the archspec repo.

dslarm commented 2 months ago

Can you add a sample of the misidentified cpu to the tests? This would allow us to unit test this failed detection in archspec

@alalazo - this is the flags excerpt from cpuinfo:

Flags:              fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asim
                    dhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 asimddp sha51
                    2 sve asimdfhm dit uscat ilrcpc flagm ssbs sb paca pacg dcpo
                    dp sve2 sveaes svepmull svebitperm svesha3 flagm2 frint svei
                    8mm svebf16 i8mm bf16 dgh rng bti

paolotricerri commented 1 month ago

Hi @alalazo, It should be safe to remove these flags from Neoverse V2 now because of the test on cpuid I propose in another PR.

What do you think?

Thanks P

paolotricerri commented 1 month ago

@alalazo , in the test folder I already see a file with data for the neoverse_V2 case. That example has got the feature that this patch is trying to remove. Do you mean adding another one taking the features from the comment above https://github.com/archspec/archspec-json/pull/106#issuecomment-2178848817 ?

alalazo commented 1 month ago

Do you mean adding another one taking the features from the comment above https://github.com/archspec/archspec-json/pull/106#issuecomment-2178848817 ?

Yes, they would be both two real cases of neoverse_v2 that we need to detect.

alalazo commented 2 weeks ago

@paolotricerri Can you add a sample of /proc/cpuinfo for the Neoverse V2 without the crypto features?

paolotricerri commented 2 weeks ago

I have moved the question I had over to https://github.com/archspec/archspec-json/pull/115 as that PR closes this one. Thanks

archspec / archspec-json

Neoverse V2: remove `sm3`, `sm4` and `svesm4` #106