armv7l should not default to NEON enabled

[redstar]

I moved the ARM buildbot slave to a different server. The new one is based on a Marvell PJ4 core. This is an ARMv7 architecture but without the NEON feature. The triple is armv7l-unknown-linux-gnueabihf, like my old krait-based board. LLVM is not aware of this CPU. In driver/targetmachine.cpp, function getARMTargetCPU(), the cortex-a8 CPU is used for the armv7l architecture. This is an inappropriate choice because this CPU has the NEON feature enabled. Every D application crashes during startup.

My work around was to use the options -mcpu=generic -mattr=+v7,-neon but this is really cumbersome.

[ximion]

We ran into this at Debian too, since all armhf builders there are without NEON support.
@markos should we add this workaround to get the armhf port working? I would be very much for doing that...

[markos]

Does this affect actual code generation? In particular is the compiler still able to output NEON code? If yes, then we can enable this for package building, otherwise, I'm not willing to cripple the compiler just because the armhf builders lack NEON. I can always do a bin-NMU from a system that works properly.

[ximion]

@markos Yes, but then you will output a compiler in the Debian archive which is unable to compile anything in Debian, which is even worse - currently, all reverse-dependencies on LDC on armhf are FTBFSing because of that.
If the compiler built without NEON could still detect it properly on platforms that support it, that would be ideal, of course.

[markos]

In that case, we could set default D_FLAGS in dpkg-buildflags that set this to all auto-builders, so that all D packages will be built without NEON by default. But I really would not want to cripple the compiler itself, because 90% of all ARM users will have a SoC with NEON and will expect to be able to use it. If we exclude this, they're just going to skip the packaged LDC.

[ximion]

We can not set global DFLAGS because LDC isn't compatible with GCCs/GDCs flags, which is a major pain.
We could try to introduce a new flag (DFLAGS_LDC maybe) and make everone support it additionally to the global DFLAGS though...

[markos]

ok, let's wait for @redstar to assure that disabling NEON in the compiler won't exclude NEON code generation entirely and if that's true, I'm not against disabling it with custom flags initially and then via some DFLAGS_LDC in dpkg-buildflags

[dnadlinger]

What does Clang do? IIRC the (micro)arch detection code was adapted from there.

[kinke]

LDC built with itself using -mattr=-neon won't use NEON instructions itself, but would still be able to emit NEON code (and I think do so by default).

[ximion]

@kinke Excellent! @markos Would you apply the change in Git and upload the package? (Or I do it later tonight)

[markos]

ok, I'll do it in a while. @kinke unfortunately by default, it built itself with neon enabled, which is why it failed on the neon-less buildds, causing the whole problem. Still, thanks for the hint!

[kinke]

Yep I know that the actual issue here is the CPU/features detection, and to fix it we'll probably have to do what clang does. Pretty messy stuff for ARM apparently...

[dnadlinger]

Pretty messy stuff for ARM apparently...

So messy that our current logic, which is supposed to be a copy of what Clang does, apparently isn't comprehensive. ;)

[kinke]

As far as I can tell, clang does things a substantially more elaborate way, see their huge Targets.cpp file. LLVM provides a llvm::ARM::getDefaultCPU(archName) function (used by clang), which uses this file to determine default ARM CPUs, FPUs etc. I've just tested it (= patched LDC, not clang) with the armv7l-unknown-linux-gnueabihf triple, and it also returns cortex-a8 and thus wouldn't solve the issue at hand.
Given the ARM variety, it may make sense to use the generic CPU (for that architecture) as default instead. At least for bootstrapping builds, -mcpu=native should get rid of these (ARM) issues for good.

[dnadlinger]

Just ran into this again, and debugged it from scratch as it has been just long enough for this to slip from memory.

The root cause seems to be that pretty much all of the ARMv7-A processors out there feature ARM Cortex-A cores, which come with NEON. Only Marvell and Nvidia seem to have rolled their own cores without NEON support. And LLVM doesn't even have an ARMv7-A CPU without NEON in ARMTargetsParser.def (armv7-a is also defined as an arch with NEON FPU), so it doesn't seem we can ever correctly auto-detect this without fixing this in LLVM (or hard-coding -neon in LDC, which doesn't even seem like the worst option).

[dnadlinger]

I believe the most sensible option is to implicitly add the -neon feature on armv7a as long as no CPU is explicitly specified (i.e. when we use LLVM's default lookup).

[JohanEngelen]

I'd prefer to go with LLVM/Clang behaviour in cases like this. See relevant discussion for Clang here:
https://bugs.llvm.org//show_bug.cgi?id=30842

Is it really necessary to pessimize codegen for armv7 by default, or could Debian just add -mattr=-neon to ldc2.conf? I can understand that it is cumbersome for users to having to add -neon, but conversely, it is also cumbersome for the other set of users to having to add +neon for performant code, right? (data needed: what are the sizes of these two sets, and what is the performance gain of +neon?)

[joakim-noah]

llvm switched to -mcpu=generic by default with the upcoming 5.0 release, such that I now have to specify cortex-a8 for Android/ARM, so as to get llvm not to error out in the loop vectorization pass. Perhaps that fixes this issue?