There was a problem hiding this comment.
Quick comment: Why not add a CALL_INTRINSIC_2 opcode (assuming we expect more of these)? The variable stack effect based on comparison with an arbitrary constant feels a little fragile. Maybe it would feel less arbitrary if the constant was actually 128 (i.e., bit 7, 0x80).
Sorry, something went wrong.
|
Quick comment: Why not add a CALL_INTRINSIC_2 opcode (assuming we expect more of these)? The variable stack effect based on comparison with an arbitrary constant feels a little fragile. Maybe it would feel less arbitrary if the constant was actually 128 (i.e., bit 7, 0x80). I considered this, the down side is we have two opcodes for non-perf-critical stuff. I don't have a strong opinion either way. @markshannon ? |
Sorry, something went wrong.
|
Is FORMAT_VALUE performance critical? It could be replaced by a couple of intrinsic functions. |
Sorry, something went wrong.
|
Is FORMAT_VALUE performance critical? It could be replaced by a couple of intrinsic functions. Possibly. Most of it could become a helper function. This would lose the fast path for f"aaa{foo}zzz" if foo is exactly a Unicode string, but I'm not sure how important that really is. |
Sorry, something went wrong.
|
Usually we want to do dispatching in the dispatch loop, not within instructions. On balance, I still think we want two instructions. The extra test and branch is messy; two instructions would make for clearer code and we aren't that short of opcodes. |
Sorry, something went wrong.
|
Regarding FORMAT_VALUE, I refer you to the venerable #6132 which split FORMAT_VALUE into CONVERT_VALUE, FORMAT_SIMPLE and FORMAT_WITH_SPEC. CONVERT_VALUE could be replaced with CALL_INSTINSIC_1 and FORMAT_WITH_SPEC could be replaced with CALL_INSTINSIC_2. FORMAT_SIMPLE is simple enough, and probably common enough, that it merits its own instruction. |
Sorry, something went wrong.
There was a problem hiding this comment.
Looks good.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.