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

cgen: better code generation for .noreturn calls #1288

Merged

Conversation

zerbina
Copy link
Collaborator

@zerbina zerbina commented Apr 28, 2024

Summary

Improve C code generation for .noreturn calls, by omitting the
unnecessary error flag check for them.

Details

If a procedure only returns due to exceptional unwinding, the
if(NIM_UNLIKELY(*nimErr_)) guard guarding the unwinding is
unnecessary, since it's guaranteed that the error flag is set (all
exits of the procedure are due to exceptional unwinding).

Unconditionally jumping to the error handling target makes it clear
to the C compiler that control-flow doesn't continue normally after
the call

Other improvements

In addition, the canRaiseConservative usage is unnecessary too, since
a cnkCheckedCall already signals that the call can raise. The
procedure has no more users and is thus removed.

Finally, for compilers that support it, a call to a .noreturn
procedure that doesn't raise is now followed by an __assume(0)
statement, communicating to the C compiler that control-flow never
returns to the caller.

Summary
=======

Improve C code generation for `.noreturn` calls, by omitting the
unnecessary error flag check for them.

Details
=======

If the procedure doesn't return via non-exceptional control flow, the
`if(NIM_UNLIKELY(*nimErr_))` guard guarding the unwinding is
unnecessary, since it's guaranteed that the error flag is set (all
exits of the procedure are due to exceptional unwinding).

In addition, the `canRaiseConservative` usage is unnecessary too, since
a `cnkCheckedCall` already signals that the call can raise. The
procedure has no more users and is thus removed.

Finally, for compilers that support it, a call to a `.noreturn`
procedure that doesn't raise is now followed by an `__assume(0)`
statement, communicating to the C compiler that control-flow never
falls out of the procedure.
@zerbina zerbina added the compiler/backend Related to backend system of the compiler label Apr 28, 2024
@zerbina zerbina added this to the C backend rework milestone Apr 28, 2024
@zerbina
Copy link
Collaborator Author

zerbina commented Apr 28, 2024

For an example of how code generation improves, consider the following:

proc test(num: int) =
  let val =
    if num == 2: 1
    elif num == 4: 2
    else: unreachable() # call to a `.noreturn` procedure

  if val >= 1:
    echo "here"

The produced C code was roughly equivalent to the following:

int* errorFlag;
void unreachable();

void test(int num) {
    int val = 0;
    if (num == 2) {
        val = 1;
    } else if(num == 4) {
        val = 2;
    } else {
        unreachable();
        if (*errorFlag) goto BeforeRet_;
    }
    if (num >= 1) {
        printf("here");
    }
    BeforeRet_:
    return;
}

(optimized assembler)

After the PR, the if(*errorFlag) guard is gone. As can be seen here, an optimizing C compiler (gcc, in this case) is then able omit the code path for the (impossible) *errorFlag == 0 case.

@saem
Copy link
Collaborator

saem commented Apr 28, 2024

/merge

Copy link

Merge requested by: @saem

Contents after the first section break of the PR description has been removed and preserved below:


Notes for Reviewers

  • a preparation for lowering the runtime check magics with a MIR pass
  • marking procedures as .noreturn previously didn't improve code generation; now it does

@chore-runner chore-runner bot added this pull request to the merge queue Apr 28, 2024
Merged via the queue into nim-works:devel with commit 46019b4 Apr 28, 2024
31 checks passed
@zerbina zerbina deleted the cgen-no-error-flag-tests-for-noreturn branch April 29, 2024 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/backend Related to backend system of the compiler
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants