Simplify `_Py_uop_sym_is_const`

[tomasr8]

Feature or enhancement

Proposal:

_Py_uop_sym_is_const and _Py_uop_sym_get_const are almost identical. The only difference AFAICS is the return type:

cpython/Python/optimizer_symbols.c

Lines 106 to 122 in 1550c30

	bool
	_Py_uop_sym_is_const(JitOptContext *ctx, JitOptSymbol *sym)
	{
	if (sym->tag == JIT_SYM_KNOWN_VALUE_TAG) {
	return true;
	}
	if (sym->tag == JIT_SYM_TRUTHINESS_TAG) {
	JitOptSymbol *value = allocation_base(ctx) + sym->truthiness.value;
	int truthiness = _Py_uop_sym_truthiness(ctx, value);
	if (truthiness < 0) {
	return false;
	}
	make_const(sym, (truthiness ^ sym->truthiness.invert) ? Py_True : Py_False);
	return true;
	}
	return false;
	}

cpython/Python/optimizer_symbols.c

Lines 131 to 148 in 1550c30

	PyObject *
	_Py_uop_sym_get_const(JitOptContext *ctx, JitOptSymbol *sym)
	{
	if (sym->tag == JIT_SYM_KNOWN_VALUE_TAG) {
	return sym->value.value;
	}
	if (sym->tag == JIT_SYM_TRUTHINESS_TAG) {
	JitOptSymbol *value = allocation_base(ctx) + sym->truthiness.value;
	int truthiness = _Py_uop_sym_truthiness(ctx, value);
	if (truthiness < 0) {
	return NULL;
	}
	PyObject *res = (truthiness ^ sym->truthiness.invert) ? Py_True : Py_False;
	make_const(sym, res);
	return res;
	}
	return NULL;
	}

We could simplify _Py_uop_sym_is_const by calling _Py_uop_sym_get_const and checking if the return is NULL:

diff --git a/Python/optimizer_symbols.c b/Python/optimizer_symbols.c
index e8a4f87031..1f58a3279e 100644
--- a/Python/optimizer_symbols.c
+++ b/Python/optimizer_symbols.c
@@ -106,19 +106,8 @@ _Py_uop_sym_is_not_null(JitOptSymbol *sym) {
 bool
 _Py_uop_sym_is_const(JitOptContext *ctx, JitOptSymbol *sym)
 {
-    if (sym->tag == JIT_SYM_KNOWN_VALUE_TAG) {
-        return true;
-    }
-    if (sym->tag == JIT_SYM_TRUTHINESS_TAG) {
-        JitOptSymbol *value = allocation_base(ctx) + sym->truthiness.value;
-        int truthiness = _Py_uop_sym_truthiness(ctx, value);
-        if (truthiness < 0) {
-            return false;
-        }
-        make_const(sym, (truthiness ^ sym->truthiness.invert) ? Py_True : Py_False);
-        return true;
-    }
-    return false;
+    PyObject *res = _Py_uop_sym_get_const(ctx, sym);
+    return res == NULL ? false : true;
 }
 
 bool

Has this already been discussed elsewhere?

This is a minor feature, which does not need previous discussion elsewhere

Links to previous discussion of this feature:

No response

Linked PRs

• gh-133342: Remove sym_is_const and sym_has_type #133884

[brandtbucher]

Yep, ditto for sym_has_type/sym_get_type.

In the interest of efficiency, though, maybe we can get rid of the sym_has_type/sym_is_const helpers entirely? A NULL return from the sym_get_* should be useful enough, and it avoids the temptation to compute something multiple times (the "check" is always followed by a "get").

For instance, _BINARY_OP_ADD_INT runs the same logic three different times for each input:

    op(_BINARY_OP_ADD_INT, (left, right -- res)) {
        if (sym_is_const(ctx, left) && sym_is_const(ctx, right)) {
            assert(PyLong_CheckExact(sym_get_const(ctx, left)));
            assert(PyLong_CheckExact(sym_get_const(ctx, right)));
            PyObject *temp = _PyLong_Add((PyLongObject *)sym_get_const(ctx, left),
                                         (PyLongObject *)sym_get_const(ctx, right));

Instead, this could just be:

    op(_BINARY_OP_ADD_INT, (left, right -- res)) {
        PyObject *lhs = sym_get_const(ctx, left);
        PyObject *rhs = sym_get_const(ctx, right);
        if (left && right) {
            assert(PyLong_CheckExact(left));
            assert(PyLong_CheckExact(right));
            PyObject *temp = _PyLong_Add((PyLongObject *)left, (PyLongObject *)right));

[tomasr8]

I was worried there would be some patterns that would make replacing these functions more awkward. For example:

if (...) {
// ...
}
else if (sym_is_const(value)) {
  PyObject *obj = sym_get_const(value);
  // ...
}

but it appears it is currently not the case. I think we can safely get rid of both functions and the code will generally become cleaner as well. We can always add either function later, it's just two lines anyway.