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

Fixes issue reported under #7217 #7228

Closed

Conversation

stuartarchibald
Copy link
Contributor

This fixes the issue with literal_unroll reported in #7217.
Does not close the issue as there's potentially more than just
this problem with literal_unroll present.

This patch adds a fix to update the scope of blocks in the
versioned loop bodies with variables that have been versioned.

This fixes the issue with ``literal_unroll`` reported in numba#7217.
Does not close the issue as there's potentially more than just
this problem with ``literal_unroll`` present.

This patch adds a fix to update the scope of blocks in the
versioned loop bodies with variables that have been versioned.
@stuartarchibald
Copy link
Contributor Author

/AzurePipelines run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 1 pipeline(s).

@stuartarchibald
Copy link
Contributor Author

NOTE: AZP is maybe having some issues. Have had to request reruns/close-open PR. Deleted comments from azure-pipelines bot are just of confirmations of request.

Comment on lines +914 to +922
new_var_table = get_name_var_table(loop_blocks)
for blk in loop_blocks.values():
scope = blk.scope
real_con = scope.localvars._con
for name, var in dict(scope.localvars._con).items():
if name in new_var_dict:
new_name = new_var_dict[name]
del real_con[name]
real_con[new_name] = new_var_table[new_name]
Copy link
Member

Choose a reason for hiding this comment

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

I probably will make a follow up PR to refactor it with ir_utils.fixup_var_define_in_scope

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I think the issue is that the versioned loop bodied have versions of some of the original variables and potentially replace the original.

@stuartarchibald
Copy link
Contributor Author

/AzurePipelines run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 1 pipeline(s).

@stuartarchibald stuartarchibald added the Effort - short Short size effort needed label Aug 3, 2021
@sklam sklam self-assigned this Nov 18, 2021
@sklam sklam added 4 - Waiting on reviewer Waiting for reviewer to respond to author and removed 3 - Ready for Review labels Nov 18, 2021
@sklam
Copy link
Member

sklam commented Nov 19, 2021

I think #7223 resolved the problem this patch is fixing. The test no longer fails on master

@sklam sklam closed this Nov 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - Waiting on reviewer Waiting for reviewer to respond to author Effort - short Short size effort needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants