Skip to content

try block prevents LLVM from identifying a throwing path is unreachable #126670

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

Closed
jeremy-rifkin opened this issue Feb 11, 2025 · 4 comments · Fixed by #127640
Closed

try block prevents LLVM from identifying a throwing path is unreachable #126670

jeremy-rifkin opened this issue Feb 11, 2025 · 4 comments · Fixed by #127640

Comments

@jeremy-rifkin
Copy link

For the following code LLVM inlines std::vector::at and optimizes away the bounds check

std::vector<int> without_try_catch() {
    std::vector<int> v(10'000);
    for(int i = 0; i < 10'000; ++i) {
        v.at(i) = i;
    }
    return v;
}

However, for the following, the presence of a try/catch precludes LLVM from proving away the bounds check and the subsequent throwing path.

std::vector<int> with_try_catch() {
    std::vector<int> v(10'000);
    for(int i = 0; i < 10'000; ++i) {
        try {
            v.at(i) = i;
        } catch(...) {}
    }
    return v;
}

As a result this dead-code try/catch results in far worse codegen.

CE link: https://godbolt.org/z/e3sT1z4bd

@frederick-vs-ja
Copy link
Contributor

#46035 seems related.

@frederick-vs-ja frederick-vs-ja added clang:codegen IR generation bugs: mangling, exceptions, etc. missed-optimization and removed new issue labels Feb 11, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 11, 2025

@llvm/issue-subscribers-clang-codegen

Author: Jeremy Rifkin (jeremy-rifkin)

For the following code LLVM inlines `std::vector::at` and optimizes away the bounds check ```cpp std::vector<int> without_try_catch() { std::vector<int> v(10'000); for(int i = 0; i < 10'000; ++i) { v.at(i) = i; } return v; } ```

However, for the following, the presence of a try/catch precludes LLVM from proving away the bounds check and the subsequent throwing path.

std::vector&lt;int&gt; with_try_catch() {
    std::vector&lt;int&gt; v(10'000);
    for(int i = 0; i &lt; 10'000; ++i) {
        try {
            v.at(i) = i;
        } catch(...) {}
    }
    return v;
}

As a result this dead-code try/catch results in far worse codegen.

CE link: https://godbolt.org/z/e3sT1z4bd

@efriedma-quic
Copy link
Collaborator

-debug-only=gvn says:

GVN: load ptr %0 is clobbered by   %5 = tail call ptr @__cxa_begin_catch(ptr %4) #15

Not sure how it's reaching that conclusion.

The aliasing between the "catch" and the vector ultimately prevents analyzing the vector: we can't prove the vector isn't reallocated by the catch. (It's as if the user wrote something like catch(...) {v = foo();}).

@nikic
Copy link
Contributor

nikic commented Feb 14, 2025

Not sure how it's reaching that conclusion.

This is because we're not treating extractvalue as an escape source. It would make sense to do that as long as CaptureTracking doesn't try to track captures through aggregates.

@nikic nikic self-assigned this Feb 14, 2025
@nikic nikic added llvm:optimizations and removed clang:codegen IR generation bugs: mangling, exceptions, etc. labels Feb 18, 2025
nikic added a commit that referenced this issue Feb 18, 2025
nikic added a commit to nikic/llvm-project that referenced this issue Feb 18, 2025
CaptureTracking considers insertions into aggregates and vectors
as captures. As such, extractions from aggregates and vectors are
escape sources. A non-escaping identified local cannot alias with
the result of an extractvalue/extractelement.

Fixes llvm#126670.
wldfngrs pushed a commit to wldfngrs/llvm-project that referenced this issue Feb 19, 2025
@nikic nikic closed this as completed in 850062c Feb 19, 2025
github-actions bot pushed a commit to arm/arm-toolchain that referenced this issue Feb 19, 2025
…urces (#127640)

CaptureTracking considers insertions into aggregates and vectors as
captures. As such, extractions from aggregates and vectors are escape
sources. A non-escaping identified local cannot alias with the result of
an extractvalue/extractelement.

Fixes llvm/llvm-project#126670.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment