compileopts: adapt GOARM to follow new 1.22 scheme #4189
Conversation
There was a problem hiding this comment.
You need to add a new test case to builder/builder_test.go, there's a list of GOOS/GOARCH/GOARM combinations that get tested. Once you do that, you can test these flags using make test. This will tell you the exact LLVM features you need to use.
This currently tests all of TinyGo and takes a while, you can make a modification similar to #4190 to only test the builder package.
|
It might be useful to try out combinations of flags using Godbolt: https://godbolt.org/z/9cGfzhG6x |
MDr164
force-pushed
the
goarm-1.22
branch
5 times, most recently
from
be4f53e
to
761b18b
Compare
This ads the optional softfloat and hardfloat suffix to GOARM the same way it was introduced upstream with Go 1.22. By default armv5 is softfloat while v6 and v7 are hardfloat. Signed-off-by: Marvin Drees <[email protected]>
Signed-off-by: Marvin Drees <[email protected]>
|
Looks like CI is failing for code I didn't even touch in this PR. Otherwise this looks fine now. Will have to run a few more tests to be certain all the flags and targets check out on my hardware here but I think it's ready to go. |
There was a problem hiding this comment.
If I read this PR correctly, it will now default to softfloat everywhere except when hardfloat is specified explicitly? Is that true? That seems like a bad idea. A better default would be to use softfloat on armv5 and hardfloat on armv6 and armv7 (or softfloat on armv5+armv6 and hardfloat on armv7).
Also, I don't think this affects musl, so you will likely still have some floating point instructions (or ABI incompatibilities). Try running this on an ARM system:
GOARM=5,softfloat tinygo run ./testdata/math.go
GOARM=5,hardfloat tinygo run ./testdata/math.go
GOARM=7,softfloat tinygo run ./testdata/math.go
GOARM=7,hardfloat tinygo run ./testdata/math.go
They should all produce identical output.
| spec.CFlags = append(spec.CFlags, "-mfloat-abi=hard") | ||
| spec.Features = "+armv5t,+strict-align,-aes,-bf16,-d32,-dotprod,-fp-armv8,-fp-armv8d16,-fp-armv8d16sp,-fp-armv8sp,-fp16,-fp16fml,-fp64,-fpregs,-fullfp16,-mve.fp,-neon,-sha2,-thumb-mode,-vfp2,-vfp2sp,-vfp3,-vfp3d16,-vfp3d16sp,-vfp3sp,-vfp4,-vfp4d16,-vfp4d16sp,-vfp4sp" | ||
| } else { | ||
| spec.CFlags = append(spec.CFlags, "-mfloat-abi=soft") // maybe use softfp instead? |
There was a problem hiding this comment.
softfp means to still use the FPU (but use an ABI compatible with soft-float code), which is not what we want here. So this comment can be removed.
| spec.CFlags = append(spec.CFlags, "-mfloat-abi=soft") // maybe use softfp instead? | |
| spec.CFlags = append(spec.CFlags, "-mfloat-abi=soft") |
Same goes for the other two similar comments below.
This ads the optional softfloat and hardfloat suffix to GOARM the same way it was introduced upstream with Go 1.22. By default armv5 is softfloat while v6 and v7 are hardfloat. Resolves #4177
TODO: Actually figure out the correct combination of LLVM arch flags.