Skip to content

Avoid creating locals for unrechable terminators. #736

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

FractalFir
Copy link
Contributor

In the unrechable terminator, we often needlessly create a temporary variable. The code accessing and returning that variable ought to never execute anyway.

This PR changes cg_gcc to return a constant 0 instead when possible. This should remove the need for the temporary in most cases, and have no effect on runtime(we change dead code + returning uninitialized locals is UB anyway, so returning 0 is fine). From a cursory look, this has little to no effect on the generated assembly(when optimizations are enabled), and should reduce the memory consumption of cg_gcc slightly(no needless locals),

@antoyo
Copy link
Contributor

antoyo commented Jul 14, 2025

reduce the memory consumption of cg_gcc slightly

Have you been able to measure that?
I would be reluctant to make the code more complex if this doesn't result in a good reduction of RAM usage. I'm not sure where I would put the threshold, but I guess if it saves a few megabytes when compiling some crates, that would be good.

Also, have you tried returning __builtin_unreachable directly? My guess is that this won't work, but we never know.

@FractalFir
Copy link
Contributor Author

FractalFir commented Jul 14, 2025

reduce the memory consumption of cg_gcc slightly

Have you been able to measure that? I would be reluctant to make the code more complex if this doesn't result in a good reduction of RAM usage. I'm not sure where I would put the threshold, but I guess if it saves a few megabytes when compiling some crates, that would be good.

Also, have you tried returning __builtin_unreachable directly? My guess is that this won't work, but we never know.

I haven't really measured the difference - I am going off libgccjit code. Maybe I am going off the wrong direction here, but I am just looking for "easy" ways to avoid needless allocations.

https://github.com/rust-lang/gcc/blob/df5343150b1791e38762da56b5647dbe0b080174/gcc/jit/jit-recording.cc#L4617

We allocate a new recorded string with the contents "unreachableReturn" - that is (length 17 + string itself 16 = 33 bytes) down the drain for just the local name. When you take into account the size of the "local" type, and other sources of overhead, it just starts to accumulate.

@antoyo
Copy link
Contributor

antoyo commented Jul 14, 2025

I guess a better solution would be to just drop a call to the builtin like you could do in C:

int get_val() {
    __builtin_unreachable();
}

But I guess libgccjit would complain about a non-terminated block. So, we would need something like Block::end_with_unreachable, which would be more difficult to implement.

Have you tried the solution suggested in my previous post?

@FractalFir
Copy link
Contributor Author

gcc_jit_block_end_with_return: mismatching types: return of __builtin_unreachable () (type: void) in function _ZN37_$LT$u8$u20$as$u20$mini_core..Mul$GT$3mul17h6aef42af51765418E (return type: __uint8_t)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants