Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add armv8* arch variants to rpmrc.in #425

Merged
merged 1 commit into from
Oct 1, 2018

Conversation

berolinux
Copy link
Contributor

armv8* is aarch64 machines in 32-bit mode

Signed-off-by: Bernhard Rosenkränzer bero@lindev.ch

rpmrc.in Outdated
@@ -461,6 +472,8 @@ arch_compat: armv5tl: armv4tl
arch_compat: armv4tl: armv4l
arch_compat: armv4l: armv3l
arch_compat: armv3l: noarch
arch_compat: armv8hnl: armv8hl
arch_compat: armv8hl: armv7hnl
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not compatible. This does not describe a true superset to subset case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since NEON is mandatory on aarch64, armv8hnl and armv8hl are the same thing, so it does describe a true superset to subset case. (To be even more correct, should probably add arch_compat: armv8hl: armv8hnl -- but I don't think circular loops there are a good idea.)

It may be worth arguing armv8hnl shouldn't exist at all (because armv8hl is the same and therefore the extra n is superfluous) or armv8hl shouldn't exist at all (because armv8hnl is the same and having the n in there is consistent with what was done on armv7) -- I've put in both for now to be compatible with what either side would expect.

rpmrc.in Outdated
@@ -593,6 +607,8 @@ buildarch_compat: armv4tl: armv4l
buildarch_compat: armv4l: armv3l
buildarch_compat: armv3l: noarch

buildarch_compat: armv8hnl: armv8hl
buildarch_compat: armv8hl: armv7hnl
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not compatible. This does not describe a true superset to subset case.

rpmrc.in Outdated
@@ -80,6 +80,9 @@ optflags: armv6hl -O2 -g -march=armv6 -mfloat-abi=hard -mfpu=vfp
optflags: armv7l -O2 -g -march=armv7
optflags: armv7hl -O2 -g -march=armv7-a -mfloat-abi=hard -mfpu=vfpv3-d16
optflags: armv7hnl -O2 -g -march=armv7-a -mfloat-abi=hard -mfpu=neon
optflags: armv8l -O2 -g -march=armv8-a
optflags: armv8hl -O2 -g -march=armv8-a -mfloat-abi=hard -mfpu=neon
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

armv8hl should not have neon.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, since NEON is mandatory for ARMv8 (that's why there is no -mfpu= switch in the aarch64-* compilers), armv8hl and armv8hnl are the same and -mfpu=neon will do the right thing.

I don't see any use case for the combination of armv8-a and -mfpu=something other than neon, except in the softfloat case (armv8l) if you need to use a binary-only library that still doesn't support hardfloat (Android...)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically speaking, NEON is mandatory for the 64-bit ISA, but as far as I'm aware, that is not the case for the 32-bit ISA. AArch32 v8 still allows NEON to be omitted.

@berolinux
Copy link
Contributor Author

According to http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0024a/CJHECGIH.html, "Both floating-point and NEON are required in all standard ARMv8 implementations", but "implementations targeting specialized markets may support the following combinations:
No NEON or floating-point." -- so armv8hl without n can make some sense for specialized CPU variants. Should probably strongly encourage armv8hnl w/ buildarchtranslate though, given mostly everyone will target standard armv8...

Copy link
Member

@Conan-Kudo Conan-Kudo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Nice job! 👍

Copy link
Member

@Conan-Kudo Conan-Kudo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small suggestions.

rpmrc.in Outdated
@@ -80,6 +80,9 @@ optflags: armv6hl -O2 -g -march=armv6 -mfloat-abi=hard -mfpu=vfp
optflags: armv7l -O2 -g -march=armv7
optflags: armv7hl -O2 -g -march=armv7-a -mfloat-abi=hard -mfpu=vfpv3-d16
optflags: armv7hnl -O2 -g -march=armv7-a -mfloat-abi=hard -mfpu=neon
optflags: armv8l -O2 -g -march=armv8-a
optflags: armv8hl -O2 -g -march=armv8-a -mfloat-abi=hard -mfpu=vfpv4-d16
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest changing this to -mfpu=vfpv4

rpmrc.in Outdated
@@ -80,6 +80,9 @@ optflags: armv6hl -O2 -g -march=armv6 -mfloat-abi=hard -mfpu=vfp
optflags: armv7l -O2 -g -march=armv7
optflags: armv7hl -O2 -g -march=armv7-a -mfloat-abi=hard -mfpu=vfpv3-d16
optflags: armv7hnl -O2 -g -march=armv7-a -mfloat-abi=hard -mfpu=neon
optflags: armv8l -O2 -g -march=armv8-a
optflags: armv8hl -O2 -g -march=armv8-a -mfloat-abi=hard -mfpu=vfpv4-d16
optflags: armv8hnl -O2 -g -march=armv8-a -mfloat-abi=hard -mfpu=neon
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There appears to be an armv8 variant of the neon fpu mode, so maybe we should use it? -mfpu=neon-fp-armv8

@kwizart
Copy link

kwizart commented Apr 3, 2018

armv8* is aarch64 machines in 32-bit mode.

What is aarch32 then ? (open question).

@@ -80,6 +80,10 @@ optflags: armv6hl -O2 -g -march=armv6 -mfloat-abi=hard -mfpu=vfp
optflags: armv7l -O2 -g -march=armv7
optflags: armv7hl -O2 -g -march=armv7-a -mfloat-abi=hard -mfpu=vfpv3-d16
optflags: armv7hnl -O2 -g -march=armv7-a -mfloat-abi=hard -mfpu=neon
optflags: armv8l -O2 -g -march=armv8-a
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you implicitly really means -mfloat-abi=soft ? I anyone still care about this today ?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aarch32 is the new marketing name for what we used to call the 'arm instruction set'. armv8 is the most current incarnation of the 32-bit ARM instruction set.

While the armv8 spec strongly suggests the inclusion of the VFP/Neon and other instructions. It is not strictly required. So yes, softfloat is still being used and is needed in some cases.

There are multiple ARM licenses who claim to have armv8 compatible CPUs that do not have floating point units. (Note, all of the cases I know of these are 32-bit CPUs -- and all are geared toward embedded.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kwizart yes, implicit still is -mfloat-abi=soft (both in clang and gcc) even though I haven't seen that used on any general purpose CPU in ages -- nobody wants to change what everyone has come to expect.

@berolinux
Copy link
Contributor Author

@n3npq At least in the arm world it's somewhat less insane: uname -m shows what it should, armv7 vs armv8 vs aarch64 etc. is 100% consistent across all processors regardless of manufacturer and marketing names.

@mlschroe
Copy link
Contributor

mlschroe commented Apr 5, 2018

Pardon the dumb question, but if armv8 is just the 32bit mode of aarch64 why doesn't aarch64 also need those float/neon/crypto modifiers?

@berolinux
Copy link
Contributor Author

@mlschroe aarch64 and armv8 are the same CPU, but (obviously, given one is 32bit and one is 64bit) a different ABI -- no need to carry over the legacy float ABI.

For neon, all aarch64 CPUs made so far have it, and it is part of the aarch64 core - however, ARM's docs say "Both floating-point and NEON are required in all standard ARMv8 implementations. However, implementations targeting specialized markets may support the following combinations:
No NEON [...]"

Neither gcc nor clang support aarch64 ABI without NEON for now, so targeting such a CPU is hypothetical anyway.
So far, the only specialized implementations were 32-bit anyway, but we can't rule out a special purpose aarch64 CPU without neon coming out at some point.
Given those (if they ever appear) will be rare and non-standard, if they're worth supporting at all they should probably get a modifier instead of discouraging people targeting aarch64 from using neon.

For crypto, a modifier may well be useful, aarch64 CPUs exist both with and without the crypto extensions.
Do we want to go there even though we don't have something comparable in the x86_64 world where it may make sense to have modifiers for SSE4, AVX, AVX2, SSE4a, SSE4.1, SSE4.2, .....?

@mhatle
Copy link

mhatle commented Apr 5, 2018

@berolinux Just to be clear, I -have- a customer who has asked for armv8 WITHOUT NEON/VFP. This was obviously in the embedded space, but the same customer is saying that it's not for compatibility, but is actually a 32-bit 'armv8' processor. (No 64-bit support..) I have no idea if those claims are true or not, as I've not directly worked on that processor. Thus the mass confusion continues.

But I agree with you, all aarch64 CPUs I have seen include all of the prescribed hardware as indicated by the spec. There may be some instruction scheduling differences, but so far the instruction set has been consistent and compatible.

But with that said, I agree with what @n3npq mentioned above. Having these indicates as part of the package 'name' (other then for human readable purposes) really no longer makes sense. Between compatibility frameworks, emulators, etc... the system and users should be able to specify compatibility and priority and just 'go from there'. Same with package generation. It would make my life easier (using RPM in embedded systems) to get rid of this rpmrc notion for anything more then 'human readable'.

armv8* is aarch64 machines in 32-bit mode
Also fix %arm macro to include newer processor types

Signed-off-by: Bernhard Rosenkränzer <bero@lindev.ch>
@pmatilai
Copy link
Member

My largely uneducated take in this is... how about we just forget the exception clause about specialized environments and just define armv8 in rpm to mean standard armv8?

@mhatle
Copy link

mhatle commented Apr 17, 2018

@pmatilai someone brought to my attention today there is various confusion as to what 'armv8' actually means (for a package type).

Does it mean traditional 32-bit ABI using instructions available on an 'ARMv8' processor?

or does it mean using ILP32 ABI using all of the aarch64 instructions?

In the former case, the ABI is compatible with armv7 and older.. in the later case, it's a new ABI.

If you can make it clear ARMv8 is just the compatible with everything else version -- then it's just another marker... someone can add something new for ilp32 mode, if it is ever implemented.

@berolinux
Copy link
Contributor Author

I think this makes most sense (and is what I implemented in the original patch):
armv8l == old ABI compatible with armv7l
armv8hl == old hardfloat EABI compatible with armv7hl
armv8hnl == armv8hl + neon, compatible with armv7hnl
armv8hcnl == armv8hnl + crypto extensions

ILP32, if properly implemented, probably needs a new name (if not a variant of aarch64, given it's closer to aarch64 than to armv7hnl? I'd be leaning towards aarch64_32 or something like that)

@pmatilai pmatilai merged commit b1f81b8 into rpm-software-management:master Oct 1, 2018
@pmatilai
Copy link
Member

pmatilai commented Oct 1, 2018

Since neither objections or anything new has come up in all this time, about time we merged this.

Thanks for the patch and sorry for taking so long.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants