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

Bounds check is not eliminated ssize vs size #63126

Closed
Kojoley opened this issue Jun 5, 2023 · 2 comments
Closed

Bounds check is not eliminated ssize vs size #63126

Kojoley opened this issue Jun 5, 2023 · 2 comments

Comments

@Kojoley
Copy link
Contributor

Kojoley commented Jun 5, 2023

#define _GLIBCXX_ASSERTIONS
#include <vector>

void foo(std::vector<double> &vec)
{
   auto count = ssize(vec);
   for (decltype(count) i = 0; i < count; ++i)
     vec[i] += 1;
}

-mllvm -enable-constraint-elimination does not help.
With size instead of ssize bounds check is eliminated.
https://godbolt.org/z/f5Gn9eKrc

@fhahn
Copy link
Contributor

fhahn commented Oct 20, 2023

To catch this in constraint-elimination, we need to use the info that the phi node will always be signed positive (due to the increment not wrapping). Looking.

@fhahn
Copy link
Contributor

fhahn commented Nov 20, 2023

Put up PR (#72879) to solve this in Constraintelimination, but this is now also handled by IndVarSimplify

@fhahn fhahn closed this as completed Nov 20, 2023
fhahn added a commit to fhahn/llvm-project that referenced this issue Nov 21, 2023
Use isKnownNonNegative for SLT information transfer. This can improve
results, in cases where ValueTracking can infer additional non-negative
info, e.g. for phi nodes.

This allows simplifying the check from
llvm#63126 by
ConstraintElimination. It is also simplifyed by IndVarSimplify now; note
the changes in llvm/test/Transforms/PhaseOrdering/loop-access-checks.ll,
due to this now being simplified earlier.
fhahn added a commit that referenced this issue Nov 21, 2023
Use isKnownNonNegative for information transfer. This can improve
results, in cases where ValueTracking can infer additional non-negative
info, e.g. for phi nodes.

This allows simplifying the check from
#63126 by
ConstraintElimination. It is also simplified by IndVarSimplify now; note
the changes in llvm/test/Transforms/PhaseOrdering/loop-access-checks.ll,
due to this now being simplified earlier.
qihangkong pushed a commit to rvgpu/llvm that referenced this issue Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants