GH-115457: Support splitting and replication of micro ops. - #115558
Conversation
Sorry, something went wrong.
|
Also, I like how clean this is in the DSL! |
Sorry, something went wrong.
Sorry, something went wrong.
| else if (oparg < _PyUop_Replication[opcode]) { | ||
| buffer[pc].opcode = opcode + oparg + 1; | ||
| } | ||
| else if (opcode == _JUMP_TO_TOP || opcode == _EXIT_TRACE) { |
There was a problem hiding this comment.
Maybe use op_is_end here?
Sorry, something went wrong.
There was a problem hiding this comment.
It's a static function in another file.
Sorry, something went wrong.
There was a problem hiding this comment.
We'll leave it up to Ken Jin to turn it into a macro or static inline in a header. (Because he's adding another opcode that could end the list of opcodes, a new JUMP variant.)
Sorry, something went wrong.
Fidget-Spinner
left a comment
There was a problem hiding this comment.
Looks good in general, just one minor comment.
Sorry, something went wrong.
| properties=compute_properties(op), | ||
| ) | ||
| if effect_depends_on_oparg_1(op) and "split" in op.annotations: | ||
| result.properties.oparg_and_1 = True |
There was a problem hiding this comment.
For code consistency, shouldn't this be compute_properties?
Sorry, something went wrong.
There was a problem hiding this comment.
Shouldn't what be compute_properties?
Sorry, something went wrong.
There was a problem hiding this comment.
Woops I meant shouldn't this be computed in compute_properties ?
Sorry, something went wrong.
There was a problem hiding this comment.
The oparg_and_1 property only applies to the base uop, not the replicas as their behavior does not depend on oparg & 1. compute_properties computes the properties from the definition only, so we would need to modify oparg_and_1 anyway.
Sorry, something went wrong.
gvanrossum
left a comment
There was a problem hiding this comment.
Cool!
Sorry, something went wrong.
| else if (oparg < _PyUop_Replication[opcode]) { | ||
| buffer[pc].opcode = opcode + oparg + 1; | ||
| } | ||
| else if (opcode == _JUMP_TO_TOP || opcode == _EXIT_TRACE) { |
There was a problem hiding this comment.
We'll leave it up to Ken Jin to turn it into a macro or static inline in a header. (Because he's adding another opcode that could end the list of opcodes, a new JUMP variant.)
Sorry, something went wrong.
|
The refleaks buildbot failed on test.test_capi.test_opt.TestUops.test_confidence_score. Likely causes: GH-114142, GH-115558, or GH-115688 Edit: it's likely to be fixed in #115728 |
Sorry, something went wrong.
Adds the split and replicate(N) annotations
split splits uops into two depending on the low bit of the oparg. This removes a few jumps from uops like _LOAD_ATTR_INSTANCE_VALUE.
replicate(N) replicates the original uop for each oparg in range(N). This is particularly valuable for uops that loop over oparg, but it is also useful to inline the oparg at build time rather than when patching.