Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Find a more elegant replacement for the codegen_simple_intrinsic macro #5

Open
danielsn opened this issue Mar 29, 2021 · 2 comments
Open
Labels
[I] Refactoring / Clean Up Refactoring or cleaning up of existing code

Comments

@danielsn
Copy link
Contributor

Relevant comment in intrinsics.rs

        // Codegens a simple intrinsic: ie. one which maps directly to a matching goto construct
        // We need to use this macro form because of a known limitation in rust
        // `codegen_simple_intrinsic!(self.get_sqrt(), Type::float())` gives the error message:
        //   error[E0499]: cannot borrow `*self` as mutable more than once at a time
        //    --> src/librustc_codegen_llvm/gotoc/intrinsic.rs:76:63
        //    |
        // 76 |                 codegen_simple_intrinsic!(self.get_sqrt(), Type::double())
        //    |                 ---- ------------------------                 ^^^^ second mutable borrow occurs here
        //    |                 |    |
        //    |                 |    first borrow later used by call
        //    |                 first mutable borrow occurs here

        //  To solve this, we need to store the `self.get_sqrt()` into a temporary variable.
        //  Using the macro form allows us to keep the call as a oneliner, while still making rust happy.
@adpaco-aws
Copy link
Contributor

Came across this while working on #727

@model-checking/rmc-devs do we still want to replace this macro?

I think the approach is not so bad and it allows us to handle all "builtin" intrinsics with a one-line call to this macro:

 "fmaf32" => codegen_simple_intrinsic!(Fmaf),

But maybe I'm missing something. Thoughts?

@tedinski
Copy link
Contributor

tedinski commented Jan 5, 2022

My general thought is it'd be nice not to use macros at all in this code, but this is a low priority, there are 13 of them in there, and I don't think this one in particular is a special problem as far as I can tell.

@celinval celinval added [I] Refactoring / Clean Up Refactoring or cleaning up of existing code and removed Cat: rework labels Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[I] Refactoring / Clean Up Refactoring or cleaning up of existing code
Projects
None yet
Development

No branches or pull requests

4 participants