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

Don't version BNDCHK based on an IV if the loop test is backwards #7250

Merged
merged 1 commit into from
Feb 9, 2024

Conversation

jdmpapin
Copy link
Contributor

@jdmpapin jdmpapin commented Feb 6, 2024

For an increasing loop-driving IV, the loop test must impose an upper bound, e.g. while (i < n), whereas for a decreasing loop-driving IV, the loop test must impose a lower bound instead, e.g. while (i >= n). Otherwise, the predicted maximum iteration count will be wrong, which could allow a bound check to be incorrectly skipped.

Loop versioner will now check to ensure that the direction of the loop test agrees with the sign of the step of the loop-driving IV.

Fixes eclipse-openj9/openj9#18803

For an increasing loop-driving IV, the loop test must impose an upper
bound, e.g. while (i < n), whereas for a decreasing loop-driving IV, the
loop test must impose a lower bound instead, e.g. while (i >= n).
Otherwise, the predicted maximum iteration count will be wrong, which
could allow a bound check to be incorrectly skipped.

Loop versioner will now check to ensure that the direction of the loop
test agrees with the sign of the step of the loop-driving IV.
@jdmpapin
Copy link
Contributor Author

jdmpapin commented Feb 6, 2024

@vijaysun-omr, would you mind reviewing?

@vijaysun-omr
Copy link
Contributor

jenkins build all

@vijaysun-omr
Copy link
Contributor

jenkins build riscv

@vijaysun-omr
Copy link
Contributor

Failures look to be infra related. The change itself looks conservative/safe to me and in common code. Merging.

@vijaysun-omr vijaysun-omr merged commit 92e1304 into eclipse:master Feb 9, 2024
15 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants