Use -mfpu=vfpv3 instead of -march=native for cross compilation sdks.

[flackr]

Some compiler environments (e.g. cross compilation sdk) do not support -march=native so we should test for support before adding the flag.

[dschuff]

Hm, I actually don't think we should automatically have -march=native in here at all; even when not cross-compiling, you risk generating a binary that won't run on the deployed machine. It's something the user should be able to do (i.e. we should support adding extra cflags in the build), but not something we should do automatically. So to fix your issue specifically, I guess I'd prefer a PR that removes it entirely.

[dschuff]

(We could replace it with something like -mfpu=neon which would enable NEON. That seems to have been the original intention, and should be safe, as practically every ARM chip that is likely to run wasm should have that.

[flackr]

Sure, that works. Done.

[dschuff]

Could we just remove this comment altogether now?

[dschuff]

Just the one nit, and then LGTM. @kripken do you have any objections or better ideas?

[kripken]

Looks ok to me (but not my area of expertise).

[sunfishcode]

Does -mfpu=neon affect the handling of subnormals in the implementation of floating-point operators?

[dschuff]

What I wanted from this particular change was to go from an essentially nondeterministic build (i.e. it depends on whatever the host machine has) to something that's at least reproduceable. If I remember correctly, ARMv7+NEON always uses FTZ for vectors, but I'm not sure about scalars, presumably it's the same. So it may not be compliant with wasm in that respect. I think vfpv4 would probably work. It would rule out Cortex-A9-class hardware which IIRC is very common, but realistically, because of the zoo of configurations, anyone doing an ARM build probably has to have a particular set of configurations in mind, and at least I don't have one. Perhaps @flackr or whoever added this code does?

[dschuff]

actually vfpv3 supports denormals too, and it looks like vfpv4 just adds half-precision and FMA. It looks like if you use -mfpu=vfpv3 it assumes 32 D registers, and FPUs which also support NEON have that many. If we use -mfpu=vfpv3-d16 then it includes e.g. cortex-a9 cpus without NEON. I think pretty much everything these days supports NEON. That was almost the case when we decided several years ago that NaCl on ARM would require NEON support, so it should be pretty safe now.

So yeah in summary, let's make this -mfpu=vfpv3.

[gczuczy]

May I ask whether this has been merged?

FreeBSD/aarch64 hit this issue, and the relevant bug report is https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=225600 .

So, this would most probably solve the current issue with the FreeBSD port.

[flackr]

Apologies for the delay - I didn't notice we had arrived at a resolution. I've adopted the suggested flag and dropped the comment.

In my case I'm targeting a raspberry pi, building using a cross compilation sdk from a high end x86_64 desktop.

[jbeich]

@gczuczy, aarch64 doesn't support any -mfpu= value e.g.,

$ gcc7 -mfpu=vfpv3 test.c
gcc7: error: unrecognized command line option '-mfpu=vfpv3'
$ clang60 -mfpu=vfpv3 test.c
clang-6.0: warning: argument unused during compilation: '-mfpu=vfpv3' [-Wunused-command-line-argument]

[jbeich]

@flackr, can you squash commits into one and rebase against master branch? I plan #1438 to depend on this one but it doesn't look pretty atm.

[flackr]

Done.

[kripken]

Thanks!