Skip to content

[py] Fix loop invariant attribute. #2767

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: main
Choose a base branch
from

Conversation

boschmitt
Copy link
Collaborator

Description

Python's for-loops are not guaranteed to be invariant. The AST bridge would need to do further analysis to guarantee the property. This PR does not add such an analysis, but fixes the places where the bridge might wrongly label a for-loop invariant when, in reality, it might not be.

Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

github-actions bot pushed a commit that referenced this pull request Mar 31, 2025
bodyBuilder,
startVal=startVal,
stepVal=stepVal,
isDecrementing=isDecrementing)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this must be normalized. Is the cc-loop-normalize pass being run?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hard to know. The pass does get run in various places but I'm not sure if it cover all paths used to execute Python kernels. Is this pass a requirement in all cases or only when doing other loop transformations ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normalization transforms the iteration space of a loop to a counted or invariant form.

More precisely, if we have a loop like

for (i = start_expr; comparison(i, stop_expr); i = step_expr(i)) ....

this loop in normalized form is more like

for (i' = 0; i' < new_uppser_bound_expr; i' = i' + 1) {
   i = some_expr(i');
   ....
 } 

Once in this form, the loop is much more amenable to loop transformations such as unrolling.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it is not a requirement for loops that are already normalized, but if Python is creating a loop that descends or uses a step other than 1, the loop must be normalized.

Normalization ought to be part of the new "classical optimizations" pipeline by default.

Python's for-loops are not guaranteed to be invariant. The AST bridge
would need to do further analysis to guarantee the property. This PR
does not add such an analysis, but fixes the places where the bridge
might wrongly label a for-loop invariant when, in reality, it might not
be.

Signed-off-by: boschmitt <[email protected]>
Copy link

github-actions bot commented Apr 1, 2025

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

github-actions bot pushed a commit that referenced this pull request Apr 1, 2025
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