-
-
Notifications
You must be signed in to change notification settings - Fork 34.5k
gh-130415: Narrowing to constants in branches involving is comparisons with a constant
#143895
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
181d096
e30a46b
eba715e
8834e12
d828f8c
24ac326
8c95c5f
d0a3ef7
33b8920
e9548a2
156b0df
b84abc3
222c3a7
ceaf091
fb0f8dc
4601243
142eff4
c6f4a6a
d0f799f
d98015f
21997bf
bfaa65b
b6c4019
d13b9a3
23ae776
9e34678
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,6 +39,7 @@ typedef enum _JitSymType { | |
| JIT_SYM_TUPLE_TAG = 8, | ||
| JIT_SYM_TRUTHINESS_TAG = 9, | ||
| JIT_SYM_COMPACT_INT = 10, | ||
| JIT_SYM_PREDICATE_TAG = 11, | ||
| } JitSymType; | ||
|
|
||
| typedef struct _jit_opt_known_class { | ||
|
|
@@ -71,6 +72,19 @@ typedef struct { | |
| uint16_t value; | ||
| } JitOptTruthiness; | ||
|
|
||
| typedef enum { | ||
| JIT_PRED_IS, | ||
| // JIT_PRED_EQ, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Either remove this, or add a TODO note that we'll be adding |
||
| } JitOptPredicateKind; | ||
|
|
||
| typedef struct { | ||
| uint8_t tag; | ||
| uint8_t kind; | ||
| bool invert; | ||
| uint16_t subject; | ||
| uint16_t constant; | ||
| } JitOptPredicate; | ||
|
|
||
| typedef struct { | ||
| uint8_t tag; | ||
| } JitOptCompactInt; | ||
|
|
@@ -83,6 +97,7 @@ typedef union _jit_opt_symbol { | |
| JitOptTuple tuple; | ||
| JitOptTruthiness truthiness; | ||
| JitOptCompactInt compact; | ||
| JitOptPredicate predicate; | ||
| } JitOptSymbol; | ||
|
|
||
| // This mimics the _PyStackRef API | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,6 +38,9 @@ typedef struct _Py_UOpsAbstractFrame _Py_UOpsAbstractFrame; | |
| #define sym_new_compact_int _Py_uop_sym_new_compact_int | ||
| #define sym_is_compact_int _Py_uop_sym_is_compact_int | ||
| #define sym_new_truthiness _Py_uop_sym_new_truthiness | ||
| #define sym_new_predicate _Py_uop_sym_new_predicate | ||
| #define sym_is_known_singleton _Py_uop_sym_is_known_singleton | ||
| #define sym_apply_predicate_narrowing _Py_uop_sym_apply_predicate_narrowing | ||
|
|
||
| extern int | ||
| optimize_to_bool( | ||
|
|
@@ -533,7 +536,16 @@ dummy_func(void) { | |
| } | ||
|
|
||
| op(_IS_OP, (left, right -- b, l, r)) { | ||
| b = sym_new_type(ctx, &PyBool_Type); | ||
| bool invert = (oparg != 0); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be both more powerful and result in cleaner code, if we left all analysis until the predicate is resolved. |
||
| if (sym_is_known_singleton(ctx, left)) { | ||
| b = sym_new_predicate(ctx, right, left, JIT_PRED_IS ,invert); | ||
| } | ||
| else if (sym_is_known_singleton(ctx, right)) { | ||
| b = sym_new_predicate(ctx, left, right, JIT_PRED_IS, invert); | ||
| } | ||
| else { | ||
| b = sym_new_type(ctx, &PyBool_Type); | ||
| } | ||
| l = left; | ||
| r = right; | ||
| } | ||
|
|
@@ -1142,6 +1154,7 @@ dummy_func(void) { | |
| assert(value != NULL); | ||
| eliminate_pop_guard(this_instr, value != Py_True); | ||
| } | ||
| sym_apply_predicate_narrowing(ctx, flag, true); | ||
| sym_set_const(flag, Py_True); | ||
| } | ||
|
|
||
|
|
@@ -1187,6 +1200,7 @@ dummy_func(void) { | |
| assert(value != NULL); | ||
| eliminate_pop_guard(this_instr, value != Py_False); | ||
| } | ||
| sym_apply_predicate_narrowing(ctx, flag, false); | ||
| sym_set_const(flag, Py_False); | ||
| } | ||
|
|
||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -258,6 +258,7 @@ _Py_uop_sym_set_type(JitOptContext *ctx, JitOptRef ref, PyTypeObject *typ) | |
| sym->cls.version = 0; | ||
| sym->cls.type = typ; | ||
| return; | ||
| case JIT_SYM_PREDICATE_TAG: | ||
| case JIT_SYM_TRUTHINESS_TAG: | ||
| if (typ != &PyBool_Type) { | ||
| sym_set_bottom(ctx, sym); | ||
|
|
@@ -319,6 +320,7 @@ _Py_uop_sym_set_type_version(JitOptContext *ctx, JitOptRef ref, unsigned int ver | |
| sym->tag = JIT_SYM_TYPE_VERSION_TAG; | ||
| sym->version.version = version; | ||
| return true; | ||
| case JIT_SYM_PREDICATE_TAG: | ||
| case JIT_SYM_TRUTHINESS_TAG: | ||
| if (version != PyBool_Type.tp_version_tag) { | ||
| sym_set_bottom(ctx, sym); | ||
|
|
@@ -385,6 +387,16 @@ _Py_uop_sym_set_const(JitOptContext *ctx, JitOptRef ref, PyObject *const_val) | |
| case JIT_SYM_UNKNOWN_TAG: | ||
| make_const(sym, const_val); | ||
| return; | ||
| case JIT_SYM_PREDICATE_TAG: | ||
| if (!PyBool_Check(const_val) || | ||
| (_Py_uop_sym_is_const(ctx, ref) && | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Drop these extra checks. I'm not sure they are necessary and may be creating a contradiction where none exists. If |
||
| _Py_uop_sym_get_const(ctx, ref) != const_val)) | ||
| { | ||
| sym_set_bottom(ctx, sym); | ||
| return; | ||
| } | ||
| make_const(sym, const_val); | ||
| return; | ||
| case JIT_SYM_TRUTHINESS_TAG: | ||
| if (!PyBool_Check(const_val) || | ||
| (_Py_uop_sym_is_const(ctx, ref) && | ||
|
|
@@ -538,6 +550,7 @@ _Py_uop_sym_get_type(JitOptRef ref) | |
| return _PyType_LookupByVersion(sym->version.version); | ||
| case JIT_SYM_TUPLE_TAG: | ||
| return &PyTuple_Type; | ||
| case JIT_SYM_PREDICATE_TAG: | ||
| case JIT_SYM_TRUTHINESS_TAG: | ||
| return &PyBool_Type; | ||
| case JIT_SYM_COMPACT_INT: | ||
|
|
@@ -566,6 +579,7 @@ _Py_uop_sym_get_type_version(JitOptRef ref) | |
| return Py_TYPE(sym->value.value)->tp_version_tag; | ||
| case JIT_SYM_TUPLE_TAG: | ||
| return PyTuple_Type.tp_version_tag; | ||
| case JIT_SYM_PREDICATE_TAG: | ||
| case JIT_SYM_TRUTHINESS_TAG: | ||
| return PyBool_Type.tp_version_tag; | ||
| case JIT_SYM_COMPACT_INT: | ||
|
|
@@ -759,6 +773,7 @@ _Py_uop_sym_set_compact_int(JitOptContext *ctx, JitOptRef ref) | |
| } | ||
| return; | ||
| case JIT_SYM_TUPLE_TAG: | ||
| case JIT_SYM_PREDICATE_TAG: | ||
| case JIT_SYM_TRUTHINESS_TAG: | ||
| sym_set_bottom(ctx, sym); | ||
| return; | ||
|
|
@@ -772,6 +787,64 @@ _Py_uop_sym_set_compact_int(JitOptContext *ctx, JitOptRef ref) | |
| } | ||
| } | ||
|
|
||
| JitOptRef | ||
| _Py_uop_sym_new_predicate(JitOptContext *ctx, JitOptRef subject_ref, JitOptRef constant_ref, JitOptPredicateKind kind, bool invert) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't need to restrict this to the case where one of the values is a constant. If |
||
| { | ||
| assert(_Py_uop_sym_is_const(ctx, constant_ref)); | ||
|
|
||
| JitOptSymbol *subject = PyJitRef_Unwrap(subject_ref); | ||
| JitOptSymbol *constant = PyJitRef_Unwrap(constant_ref); | ||
|
|
||
| JitOptSymbol *res = sym_new(ctx); | ||
| if (res == NULL) { | ||
| return out_of_space_ref(ctx); | ||
| } | ||
|
|
||
| res->tag = JIT_SYM_PREDICATE_TAG; | ||
| res->predicate.invert = invert; | ||
| res->predicate.kind = kind; | ||
| res->predicate.subject = (uint16_t)(subject - allocation_base(ctx)); | ||
| res->predicate.constant = (uint16_t)(constant - allocation_base(ctx)); | ||
|
|
||
| return PyJitRef_Wrap(res); | ||
| } | ||
|
|
||
| bool | ||
| _Py_uop_sym_is_known_singleton(JitOptContext *ctx, JitOptRef ref) | ||
| { | ||
| if (_Py_uop_sym_is_safe_const(ctx, ref)) { | ||
| PyObject *value = _Py_uop_sym_get_const(ctx, ref); | ||
| return value == Py_None || value == Py_True || value == Py_False; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| void | ||
| _Py_uop_sym_apply_predicate_narrowing(JitOptContext *ctx, JitOptRef ref, bool branch_is_true) | ||
| { | ||
| JitOptSymbol *sym = PyJitRef_Unwrap(ref); | ||
| if (sym->tag != JIT_SYM_PREDICATE_TAG) { | ||
| return; | ||
| } | ||
|
|
||
| JitOptPredicate pred = sym->predicate; | ||
| bool narrow = (branch_is_true && !pred.invert) || (!branch_is_true && pred.invert); | ||
| if (!narrow) { | ||
| return; | ||
| } | ||
|
|
||
| if (pred.kind == JIT_PRED_IS) { | ||
| JitOptRef subject_ref = PyJitRef_Wrap(allocation_base(ctx) + pred.subject); | ||
| JitOptRef constant_ref = PyJitRef_Wrap(allocation_base(ctx) + pred.constant); | ||
| PyObject *const_val = _Py_uop_sym_get_const(ctx, constant_ref); | ||
| if (const_val == NULL) { | ||
| return; | ||
| } | ||
| _Py_uop_sym_set_const(ctx, subject_ref, const_val); | ||
| assert(_Py_uop_sym_is_safe_const(ctx, subject_ref)); | ||
| } | ||
| } | ||
|
|
||
| JitOptRef | ||
| _Py_uop_sym_new_truthiness(JitOptContext *ctx, JitOptRef ref, bool truthy) | ||
| { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't matters whether something is a singleton or not, so you can drop this.