c2rust icon indicating copy to clipboard operation
c2rust copied to clipboard

Builtin ARM FPSCR functions not translated

Open chrysn opened this issue 4 years ago • 2 comments

The __builtin_arm_get_fpscr and __builtin_arm_set_fpscr functions recognized by LLVM/clang are not translated. Therefrom ensue errors like this:

4350 |     return __builtin_arm_get_fpscr() as uint32_t;
     |            ^^^^^^^^^^^^^^^^^^^^^^^ not found in this scope

The correct handling would probably be to add them to c2rust-transpile/src/translator/builtins.rs; don't know yet how.

[edit] fix link

chrysn avatar Aug 25 '21 15:08 chrysn

You are right that this should be handled in builtins.rs. I assume the challenge is how to call the intrinsic from Rust. Have you looked at crates such as https://github.com/aweinstock314/llvmint that expose all LLVM intrinsics (including the two you reference)? We've been hard pressed for time to work on C2Rust but we can probably get this reviewed and merged if you plan to open a PR.

thedataking avatar Aug 26 '21 20:08 thedataking

Not working on it right now, but may be come back later when this gets actually used in RIOT (right now, it's a dead function so can easily be worked around).

Two updates:

  • I don't know what I did precisely when I produced the error message (probably I was half-way trying to add it); the regular error message will be:

    error: Failed to translate __get_FPSCR: Unimplemented builtin __builtin_arm_get_fpscr
    --> /home/chrysn/git/RIOT/cpu/cortexm_common/include/vendor/cmsis_gcc.h:774:10
    
    thread 'main' panicked at 'Translation failed, see error above', c2rust-transpile/src/translator/mod.rs:499:9
    
  • The llvmint crate handles these by defining an extern function and giving it a linker name like this:

    #[link_name = "llvm.arm.get.fpscr"]
    pub fn get_fpscr() -> i32;
    

The things that make it hard enough to not immediately do it without urgent use case are:

  • The llvmint crate doesn't look like it's actively maintained.
  • The llvmint crate depends on the link_llvm_intrinsics feature (which is perma-unstable and may meet with a similar fate as llvm_asm)
  • The llvmint crate has other requirements making it impractical for my use case (unconditioinal SIMD features, not being no_std) -- but this can probably be remedied in an updated version of it.
  • Going through the link_llvm_intrinsics mechanism may incur a runtime penalty if there is no LTO (usually I'd expect a builtin to generate the assembly in the function, but I didn't check yet if there's any magic happening that makes this a non-issue).
  • There is a lot of functions that might need that, I have no clue as to how to make this work for all of them smoothly.

The changes to C2Rust would be simple (and my builtins-through-llvmint branch shows the changes that'd just need a bit of generalization if the list grows large); getting the implementations usable well (especially in a future-proof way, without using unstable Rust, which LLVM-dependent Rust will be) is not.


My course of action (and recommendation for others hitting this): Revisit where you're running into any of these builtins, check whether you actually need it, and if so, consider whether your C code is even portable. If it is, chances are there is a dispatch based on the compiler, and you could consider adding a "Rust provides me with the intrinsics" backend where they're implemented in stable Rust to the list of backends.

chrysn avatar Jan 17 '22 11:01 chrysn