Remove `mk_unique_var` in `untyped_passes.py` by guilhermeleobas · Pull Request #8266 · numba/numba

guilhermeleobas · 2022-07-20T14:44:36Z

guilhermeleobas · 2022-07-26T17:12:17Z

+                try:
+                    scope.get_exact(name)
+                except errors.NotDefinedError:
+                    # is this correct? In case the scope doesn't have the
+                    # variable, we need to define it prior creating new
+                    # copies of it!
+                    scope.define(name, var.loc)
+                new_var_dict[name] = scope.redefine(name, var.loc).name


This needs a careful look. Previously, mk_unique_var would return a unique identifier, whereas scope.redefine might just return the original identifier if it is the first time defining it.

@guilhermeleobas which tests are failing due to this?

Change this line to

for name, var in var_table.items(): scope = switch_ir.blocks[lbl].scope scope.define(name, var.loc) new_var_dict[name] = scope.redefine(name, var.loc).name

And run the tests python -m runtests numba.tests.test_mixed_tuple_unroller.TestMixedTupleUnroll.test_08

$ python -m runtests numba.tests.test_mixed_tuple_unroller.TestMixedTupleUnroll.test_08 /home/guilhermeleobas/git/numba/numba/core/utils.py:612: NumbaExperimentalFeatureWarning: First-class function type feature is experimental warnings.warn("First-class function type feature is experimental", /home/guilhermeleobas/git/numba/numba/tests/test_mixed_tuple_unroller.py:603: NumbaExperimentalFeatureWarning: First-class function type feature is experimental acc += tup2[0]() /home/guilhermeleobas/git/numba/numba/tests/test_mixed_tuple_unroller.py:605: NumbaExperimentalFeatureWarning: First-class function type feature is experimental acc += tup2[1]() /home/guilhermeleobas/git/numba/numba/tests/test_mixed_tuple_unroller.py:607: NumbaExperimentalFeatureWarning: First-class function type feature is experimental acc += tup2[2]() /home/guilhermeleobas/git/numba/numba/tests/test_mixed_tuple_unroller.py:603: NumbaExperimentalFeatureWarning: First-class function type feature is experimental acc += tup2[0]() /home/guilhermeleobas/git/numba/numba/tests/test_mixed_tuple_unroller.py:605: NumbaExperimentalFeatureWarning: First-class function type feature is experimental acc += tup2[1]() /home/guilhermeleobas/git/numba/numba/tests/test_mixed_tuple_unroller.py:607: NumbaExperimentalFeatureWarning: First-class function type feature is experimental acc += tup2[2]() E ====================================================================== ERROR: test_08 (numba.tests.test_mixed_tuple_unroller.TestMixedTupleUnroll) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/guilhermeleobas/git/numba/numba/tests/test_mixed_tuple_unroller.py", line 617, in test_08 self.assertEqual(foo(tup1, tup2), foo.py_func(tup1, tup2)) File "/home/guilhermeleobas/git/numba/numba/core/dispatcher.py", line 476, in _compile_for_args error_rewrite(e, 'interpreter') File "/home/guilhermeleobas/git/numba/numba/core/dispatcher.py", line 407, in error_rewrite raise e File "/home/guilhermeleobas/git/numba/numba/core/dispatcher.py", line 420, in _compile_for_args return_val = self.compile(tuple(argtypes)) File "/home/guilhermeleobas/git/numba/numba/core/dispatcher.py", line 965, in compile cres = self._compiler.compile(args, return_type) File "/home/guilhermeleobas/git/numba/numba/core/dispatcher.py", line 125, in compile status, retval = self._compile_cached(args, return_type) File "/home/guilhermeleobas/git/numba/numba/core/dispatcher.py", line 139, in _compile_cached retval = self._compile_core(args, return_type) File "/home/guilhermeleobas/git/numba/numba/core/dispatcher.py", line 152, in _compile_core cres = compiler.compile_extra(self.targetdescr.typing_context, File "/home/guilhermeleobas/git/numba/numba/core/compiler.py", line 716, in compile_extra return pipeline.compile_extra(func) File "/home/guilhermeleobas/git/numba/numba/core/compiler.py", line 452, in compile_extra return self._compile_bytecode() File "/home/guilhermeleobas/git/numba/numba/core/compiler.py", line 520, in _compile_bytecode return self._compile_core() File "/home/guilhermeleobas/git/numba/numba/core/compiler.py", line 499, in _compile_core raise e File "/home/guilhermeleobas/git/numba/numba/core/compiler.py", line 486, in _compile_core pm.run(self.state) File "/home/guilhermeleobas/git/numba/numba/core/compiler_machinery.py", line 368, in run raise patched_exception File "/home/guilhermeleobas/git/numba/numba/core/compiler_machinery.py", line 356, in run self._runPass(idx, pass_inst, state) File "/home/guilhermeleobas/git/numba/numba/core/compiler_lock.py", line 35, in _acquire_compile_lock return func(*args, **kwargs) File "/home/guilhermeleobas/git/numba/numba/core/compiler_machinery.py", line 311, in _runPass mutated |= check(pss.run_pass, internal_state) File "/home/guilhermeleobas/git/numba/numba/core/compiler_machinery.py", line 273, in check mangled = func(compiler_state) File "/home/guilhermeleobas/git/numba/numba/core/untyped_passes.py", line 1667, in run_pass pm.run(state) File "/home/guilhermeleobas/git/numba/numba/core/compiler_machinery.py", line 368, in run raise patched_exception File "/home/guilhermeleobas/git/numba/numba/core/compiler_machinery.py", line 356, in run self._runPass(idx, pass_inst, state) File "/home/guilhermeleobas/git/numba/numba/core/compiler_lock.py", line 35, in _acquire_compile_lock return func(*args, **kwargs) File "/home/guilhermeleobas/git/numba/numba/core/compiler_machinery.py", line 311, in _runPass mutated |= check(pss.run_pass, internal_state) File "/home/guilhermeleobas/git/numba/numba/core/compiler_machinery.py", line 273, in check mangled = func(compiler_state) File "/home/guilhermeleobas/git/numba/numba/core/untyped_passes.py", line 1296, in run_pass stat = self.apply_transform(state) File "/home/guilhermeleobas/git/numba/numba/core/untyped_passes.py", line 1160, in apply_transform self.unroll_loop(state, info) File "/home/guilhermeleobas/git/numba/numba/core/untyped_passes.py", line 1259, in unroll_loop unrolled_body = self.inject_loop_body( File "/home/guilhermeleobas/git/numba/numba/core/untyped_passes.py", line 916, in inject_loop_body scope.define(name, var.loc) File "/home/guilhermeleobas/git/numba/numba/core/ir.py", line 1107, in define self.localvars.define(v.name, v) File "/home/guilhermeleobas/git/numba/numba/core/ir.py", line 261, in define raise RedefinedError(name) numba.core.errors.RedefinedError: Failed in nopython mode pipeline (step: handles literal_unroll) Failed in literal_unroll_subpipeline mode pipeline (step: performs mixed container unroll) $const78.3 ---------------------------------------------------------------------- Ran 1 test in 0.203s FAILED (errors=1) (numba)

Thanks for the example, I think I understand what's happening. The switch_ir is some IR containing the switch table to jump to a type specific loop body version, it initially won't contain any of the variables present in the loop blocks scheduled for versioning. This part of the code is essentially wiring these versioned blocks into the switch table, which means the switch table need to "know" about the loops' variables as they are now in scope. Occasionally there are collisions, like in the case above, whereby the switch table happens to have the same name for a constant as a versioned loop body and the proposed change works around this.

I think another way to "fix" it would be to version the inlined variable names with the branch version from which they were derived, e.g.

diff --git a/numba/core/untyped_passes.py b/numba/core/untyped_passes.py --- a/numba/core/untyped_passes.py +++ b/numba/core/untyped_passes.py @@ -906,14 +906,7 @@ class MixedContainerUnroller(FunctionPass): new_var_dict = {} for name, var in var_table.items(): scope = switch_ir.blocks[lbl].scope - try: - scope.get_exact(name) - except errors.NotDefinedError: - # is this correct? In case the scope doesn't have the - # variable, we need to define it prior creating new - # copies of it! - scope.define(name, var.loc) - new_var_dict[name] = scope.redefine(name, var.loc).name + new_var_dict[name] = scope.define(f"v{branch_ty}_{name}", var.loc).name replace_var_names(loop_blocks, new_var_dict)

which might be a strategy worth employing as it makes it easier to track the origin of the variable.

Either way, I think the proposed code change is correct.

stuartarchibald

Thanks for the patch and fixes!

Remove mk_unique_var in untyped_passes.py

06b9ef0

stuartarchibald added the 2 - In Progress label Jul 20, 2022

guilhermeleobas added 4 commits July 25, 2022 20:23

Fix CI failure

78f73df

Fix another bug

5c627af

another attempt to fix ci issue

ad2879a

flake8

a11fe01

guilhermeleobas marked this pull request as ready for review July 26, 2022 17:10

guilhermeleobas requested review from sklam and stuartarchibald as code owners July 26, 2022 17:10

guilhermeleobas commented Jul 26, 2022

View reviewed changes

bszollosinagy mentioned this pull request Aug 8, 2022

Remove use of mk_unique_var in untyped_passes.py #8324

Closed

guilhermeleobas added 3 - Ready for Review and removed 2 - In Progress labels Aug 9, 2022

stuartarchibald self-assigned this Aug 9, 2022

stuartarchibald added the Effort - medium Medium size effort needed label Aug 9, 2022

esc added this to the Numba 0.57 RC milestone Aug 9, 2022

stuartarchibald added 4 - Waiting on author Waiting for author to respond to review and removed 3 - Ready for Review labels Aug 19, 2022

replace scope.redefine by scope.define

c6a2506

stuartarchibald approved these changes Aug 22, 2022

View reviewed changes

stuartarchibald added 5 - Ready to merge Review and testing done, is ready to merge and removed 4 - Waiting on author Waiting for author to respond to review labels Aug 22, 2022

sklam merged commit 1ef702a into numba:main Aug 22, 2022

stuartarchibald mentioned this pull request Mar 29, 2023

" 'compare_op' is not in scope" warning inside literal_unroll #8117

Closed

Navigation Menu

Repository navigation

Remove `mk_unique_var` in `untyped_passes.py` - #8266

Remove `mk_unique_var` in `untyped_passes.py`#8266
sklam merged 6 commits into
numba:mainfrom
guilhermeleobas:guilhermeleobas/remove_mk_unique_var_untyped_passes

guilhermeleobas commented Jul 20, 2022

Uh oh!

guilhermeleobas Jul 26, 2022

Uh oh!

stuartarchibald Aug 15, 2022

Uh oh!

guilhermeleobas Aug 15, 2022

Uh oh!

stuartarchibald Aug 19, 2022

Uh oh!

stuartarchibald left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

guilhermeleobas commented Jul 20, 2022

Uh oh!

guilhermeleobas Jul 26, 2022

Choose a reason for hiding this comment

Uh oh!

stuartarchibald Aug 15, 2022

Choose a reason for hiding this comment

Uh oh!

guilhermeleobas Aug 15, 2022

Choose a reason for hiding this comment

Uh oh!

stuartarchibald Aug 19, 2022

Choose a reason for hiding this comment

Uh oh!

stuartarchibald left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants