This enables math module for riscv32 targets

[mchodzikiewicz]

This is likely a bug fix since in math.rs there are already config attributes that are mentioning this target.

I was pointed here from esp-rs/esp-hal#881

[MabezDev]

I think we should also gate on not(target_feature = "f")

[mchodzikiewicz]

done

[bjorn3]

A fair amount of the intrinsics in libm don't have native instructions even on cpu's that do support floating point operations.

[mchodzikiewicz]

huh, my initial thinking was that if there are gates on non-fpu on relevant elements inside math.rs it is ok not to gate it on module inclusion

I'll do whatever you guys tell me, just created this PR to speed things up and have the thing going on somewhere since esp-hal was clearly not a correct place for it

[MabezDev]

A fair amount of the intrinsics in libm don't have native instructions even on cpu's that do support floating point operations.

Is there any point in gating the math module at all then? Presumably, if the backend supports code-generation instead of using an intrinsic it will just do that, and not emit a call to the intrinsic. Compile times I suppose?

[bjorn3]

Is there any point in gating the math module at all then?

Yes, we must not include it when there is already a libm implementation providing them. This libm implementation is generally part of the system libc. We are allowlisting targets without libc here.

[mchodzikiewicz]

Ok, so I brought back the original commit

[Amanieu]

Should the same be done for riscv64 bare metal targets?

[mchodzikiewicz]

good question, also does

We are allowlisting targets without libc here.

mean that we could skip architecture and go for

all(target_os = "none")

?

[Amanieu]

Yes, actually that would make more sense. It seems the same conclusion was reached for UEFI in #553, we probably shouldn't be checking target_arch at all, only the target OS.

[mchodzikiewicz]

Cool! Changed it accordingly. Maybe I should rebase on #553 already?

[Amanieu]

I've merged #553, you can rebase on top of that.

Also we should remove the target_arch checks within the math module. The aim here is that we only want to provide functions that are not already provided by the system libm, which should only depend on the OS, not the architecture.

[mchodzikiewicz]

huh, but then what about gating like all(target_arch = "riscv32", not(target_feature = "f"), target_os = "none")? should we also provide these functions for FPU-driven targets?

[Amanieu]

It doesn't hurt to provide them anyways. If they are not used then the linker will simply not include them in the output binary.

[mchodzikiewicz]

Okay, how about now?

[mchodzikiewicz]

It is not that easy - gating only on taget os "none" enables it for targets that have these symbols already leading to multiple definitions, gating module inclusion seems fine though.

Maybe some other gating scheme would do the trick, but I am not sure if it should be part of this PR