Avoid mutable arguments in CSEToAssignmentMapper#610
Conversation
kaushikcfd
force-pushed
the
do_not_allow_mutable_behavior
branch
from
8616711 to
b7f0d1c
Compare
|
/cc @isuruf (git-blame shows you had added the earlier version as a perf. improvement). |
loopy/kernel/creation.py
Outdated
| new_expression = cseam(insn.expression, frozenset()) | ||
| if contains_cse(insn.expression): |
There was a problem hiding this comment.
Maybe this condition could just become if new_expression is not insn.expression, with no need for another traversal.
There was a problem hiding this comment.
I don't think IdentityMapper is generous about returning identical expression objects.
There was a problem hiding this comment.
It should be though. There are 2 options I think,
- check for identical obejcts as @inducer said and make sure identity mapper return identical objects.
- instead of passing a found object in the call to the mapper, store in the mapper as an attribute and query it after traversal.
There was a problem hiding this comment.
I think I strongly prefer option 1.
There was a problem hiding this comment.
I don't think IdentityMapper is generous about returning identical expression objects.
Turns out I was wrong, it was already quite mature in preserving objects whenever possible.
Yep, went with option 1.
Thanks for the suggestions!
There was a problem hiding this comment.
it was already quite mature in preserving objects whenever possible.
I seem to recall @isuruf did a lot of work towards that goal, so I had a feeling it was worth a shot. Good to hear!
kaushikcfd
force-pushed
the
do_not_allow_mutable_behavior
branch
from
b7f0d1c to
6f1c948
Compare
|
LGTM, thanks! |
kaushikcfd
force-pushed
the
do_not_allow_mutable_behavior
branch
from
6f1c948 to
e1cefa2
Compare
inducer
force-pushed
the
do_not_allow_mutable_behavior
branch
from
e1cefa2 to
35a7324
Compare
3 participants
No description provided.