[mypyc] feat: ForMap generator helper for builtins.map#19649
[mypyc] feat: ForMap generator helper for builtins.map#19649BobTheBuidler wants to merge 31 commits intopython:masterfrom
builtins.map#19649Conversation
for more information, see https://pre-commit.ci
JukkaL
left a comment
JukkaL
left a comment
There was a problem hiding this comment.
Nice, map is a commonly used builtin.
| r14 = box(int, _mypyc_map_arg_0) | ||
| r15 = box(int, _mypyc_map_arg_1) | ||
| r16 = unbox(int, r14) | ||
| r17 = unbox(int, r15) |
There was a problem hiding this comment.
The boxing followed by unboxing seems pointless. It would be great if we didn't need this (it might be a pre-existing issue though).
There was a problem hiding this comment.
Agreed. I'll take a look. Does mypyc currently perform any optimization over the IR before generating the actual C code? I looked briefly at the transform module, which seems like it might play some role here, but I couldn't figure out the entrypoint, or if that section even works as I think it does
There was a problem hiding this comment.
After looking, I'm not entirely sure what causes this.
The fake NameExpr that I created is using the correct rtype in the IR, and from reading the code in for_helpers I can't figure out why (or where) r14 thru r17 are being emitted.
What I see is that we create a CallExpr using the NameExpr directly as the input, which implies to me there wouldn't be any intermediate values involved in the actual call.
There was a problem hiding this comment.
I have a PR in the works to address redundant boxing->unboxing such as this case
| L6: | ||
| return s | ||
|
|
||
| [case testForMapThreeArgs] |
There was a problem hiding this comment.
The number of irbuild tests is perhaps a bit much? I wonder if 1-2 irbuild test cases would be sufficient, with the remaining cases only covered by run tests.
There was a problem hiding this comment.
I picked 3 to keep, one-arg, 3-arg, and comprehension
for more information, see https://pre-commit.ci
| return None | ||
|
|
||
|
|
||
| @specialize_function("builtins.list") |
There was a problem hiding this comment.
This isn't supposed to be part of this PR, I'll need to do some doctoring
|
Some tests are failing. |
|
if we can merge #19948 before this one I can refactor this a bit and get rid of some LOC |
2 participants
This PR adds a
ForMaphelper class forforloops overbuiltins.mapwhich allows mypyc to maintain control of the function calling, so it can call native functions and primitive ops, and to avoid allocation of iterators in many cases.