Fix -Zregparm for LLVM builtins

[winstonallo]

This fixes the issue where -Zregparm=N was not working correctly when calling LLVM intrinsics

By default on x86-32, arguments are passed on the stack. The -Zregparm=N flag allows the first N arguments to be passed in registers instead.

When calling intrinsics like memset, LLVM still passes parameters on the stack, which prevents optimizations like tail calls.

As proposed by @tgross35, I fixed this by setting the NumRegisterParameters LLVM module flag to N when the -Zregparm=N is set.

// compiler/rust_codegen_llvm/src/context.rs#375-382
if let Some(regparm_count) = sess.opts.unstable_opts.regparm {
    llvm::add_module_flag_u32(
        llmod,
        llvm::ModuleFlagMergeBehavior::Error,
        "NumRegisterParameters",
        regparm_count,
    );
}

Here is a before/after compiler explorer.

Here is the final result for the code snippet in the original issue:

entrypoint:
        push    esi
        mov     esi, eax
        mov     eax, ecx
        mov     ecx, esi
        pop     esi
        jmp     memset   ; Tail call parameters in registers

Fixes: #145271

[winstonallo]

I think this should fix it already, let me know if I am missing something! Working on the tests now.

[rustbot]

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[winstonallo]

I added a test verifying that the memset parameters are passed in registers as discussed.

[petrochenkov]

r? nikic

[tgross35]

Thank you for doing a quick fix here!

Just a few requests to make sure we're testing what we expect

[rustbot]

This PR modifies tests/auxiliary/minicore.rs.

cc @jieyouxu

[tgross35]

Two small nits but this looks good to me once CI passes, please squash

@nikic anything else?

[winstonallo]

I squashed, let me know if there is anything else I can do!

[tgross35]

Looks great! Thanks for making this happen, and welcome :)

@bors r+

[bors]

📌 Commit 02d4666 has been approved by tgross35

It is now in the queue for this repository.

[winstonallo]

Looks great! Thanks for making this happen, and welcome :)

Thank you very much for all the help! I'll keep an eye on the issue tracker, but if you come across anything you think would be a good fit, please let me know!

[winstonallo]

Fixed the typo, thank you!

[nikic]

@bors r=tgross35 rollup=iffy assembly test

[bors]

📌 Commit 04ff144 has been approved by tgross35

It is now in the queue for this repository.

[RalfJung]

If we do this, do we have to also keep the logic in our own adjustments or does setting just the LLVM flag suffice?

[tgross35]

Regparm already sets the immarg flag, so aside from that it's reasonably opaque on our end. We are already setting immarg on libcalls, it just seems that NumRegisterParameters is required to make LLVM listen to it (not sure whether or not this is intended behavior).

Our Rust ABI isn't affected by regparm, at least not currently. In theory we probably could make use of the same settings for some perf improvements on 32-bit builds.