Repository navigation
Initial refactoring of parfor reduction lowering - #7837
Conversation
- Breakup long functions to highlight the logic structure - Replace manually constructed Numba IR with jitted python code.
| # Cleanup reduction variable | ||
| for v in redarrs.values(): | ||
| lowerer.lower_inst(ir.Del(v.name, loc=loc)) | ||
| # Restore the original typemap of the function that was replaced temporarily at the | ||
| # Beginning of this function. | ||
| lowerer.fndesc.typemap = orig_typemap | ||
|
|
||
| if config.DEBUG_ARRAY_OPT_RUNTIME: | ||
| res_print_str = "res_print" | ||
| strconsttyp = types.StringLiteral(res_print_str) | ||
| if config.DEBUG_ARRAY_OPT: | ||
| print("_lower_parfor_parallel done") |
| _parfor_lowering_finalize_reduction( | ||
| parfor, redarrs, lowerer, parfor_reddict, | ||
| ) |
There was a problem hiding this comment.
Majority of changes starts with this new function
|
@DrTodd13 please could you take an initial look at this as codeowner? Many thanks! I'm very much in favour of these changes in principle as it reduces reliance on raw IR generation. |
DrTodd13
left a comment
There was a problem hiding this comment.
I see where you are going with these changes and I appreciate the work. As most of this is conceptually just a code reorg I think it should be fine. If there are any errors introduced in the process that are subtle then I don't think humans (or at least me) are capable of spotting those problems in the code review. As long as all the tests pass then I'm happy with these changes.
stuartarchibald
left a comment
There was a problem hiding this comment.
Thanks for refactoring this @sklam, I think it's easier to follow with less reliance on code gen. I've taken a look at the changes and it seems like the new code does the equivalent of what was there previously. There's a couple of comments inline to take a look at, but as @DrTodd13 suggests, the test suite is probably a better at catching mistakes than humans trying to translate between the two styles of code generation. Thanks again!
Co-authored-by: stuartarchibald <stuartarchibald@users.noreply.github.com>
stuartarchibald
left a comment
There was a problem hiding this comment.
Thanks for the patch and fixes.
Summary:
In the long run, we want to move parfor lowering outside of the
LoweringPass.