Skip to content

gh-115419: Change default sym to not_null #116562

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Mar 13, 2024
Merged

gh-115419: Change default sym to not_null #116562

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0400f56
Eliminate guard for constant promoted function
Fidget-Spinner Mar 10, 2024
d71527b
Fix test_generated_cases
Fidget-Spinner Mar 10, 2024
ae58883
fix gH warning
Fidget-Spinner Mar 10, 2024
be261d9
simplify test
Fidget-Spinner Mar 11, 2024
4ae1f9f
address review
Fidget-Spinner Mar 11, 2024
3d28b37
add back missing thing
Fidget-Spinner Mar 11, 2024
8316cb0
Merge remote-tracking branch 'upstream/main' into constant_propagatio…
Fidget-Spinner Mar 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions Lib/test/test_capi/test_opt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -793,6 +793,53 @@ def testfunc(n):
"""))
self.assertEqual(result[0].rc, 0, result)

def test_const_eliminate_function_guards(self):
"""Note: this function must be executed in a subprocess, because when other tests are run,
the globals of this module might get affected, causing globals to constant promotion
to fail.
"""
Copy link
Member

@gvanrossum gvanrossum Mar 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I betcha you can do without a subprocess. Instead, just create a namespace and use that, like several other tests do. The namespace directory acts as the globals for the function defined in it. Try it!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry my Python is actually somewhat rusty -- which namespace are you referring to? I don't remember types.SimpleNamespace being that powerful.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a dict -- look at test_type_inconsistency() in the same file for a complete example. Because it says exec(src, ns, ns) the ns dict is used as the globals for the function, and you can add/update its "globals" using ns["some_name"] = some_value.


result = script_helper.run_python_until_end('-c', textwrap.dedent("""
import _testinternalcapi
import opcode

def get_first_executor(func):
code = func.__code__
co_code = code.co_code
JUMP_BACKWARD = opcode.opmap["JUMP_BACKWARD"]
for i in range(0, len(co_code), 2):
if co_code[i] == JUMP_BACKWARD:
try:
return _testinternalcapi.get_executor(code, i)
except ValueError:
pass
return None

def get_opnames(ex):
return {item[0] for item in ex}

def func(n):
return n

def testfunc(n):
for i in range(n):
x = func(i)
return x

opt = _testinternalcapi.new_uop_optimizer()
_testinternalcapi.set_optimizer(opt)
testfunc(64)

ex = get_first_executor(testfunc)
assert ex is not None
uops = get_opnames(ex)
print(uops)
assert "_LOAD_GLOBAL_MODULE" not in uops
assert "_LOAD_CONST_INLINE_WITH_NULL" in uops
assert "_CHECK_FUNCTION_EXACT_ARGS" not in uops
"""))
self.assertEqual(result[0].rc, 0, result)

def test_float_add_constant_propagation(self):
def testfunc(n):
a = 1.0
Expand Down
4 changes: 2 additions & 2 deletions Lib/test/test_generated_cases.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -908,7 +908,7 @@ def test_overridden_abstract_args(self):

case OP2: {
_Py_UopsSymbol *out;
out = sym_new_unknown(ctx);
out = sym_new_not_null(ctx);
if (out == NULL) goto out_of_space;
stack_pointer[-1] = out;
break;
Expand All @@ -933,7 +933,7 @@ def test_no_overridden_case(self):
output = """
case OP: {
_Py_UopsSymbol *out;
out = sym_new_unknown(ctx);
out = sym_new_not_null(ctx);
if (out == NULL) goto out_of_space;
stack_pointer[-1] = out;
break;
Expand Down
25 changes: 23 additions & 2 deletions Python/optimizer_bytecodes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,14 @@ dummy_func(void) {
}
}

op(_LOAD_ATTR, (owner -- attr, self_or_null if (oparg & 1))) {
(void)owner;
OUT_OF_SPACE_IF_NULL(attr = sym_new_not_null(ctx));
if (oparg & 1) {
OUT_OF_SPACE_IF_NULL(self_or_null = sym_new_unknown(ctx));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At some point in the future we will be able to tell whether the attribute is a method or not, and hence we will be able to set it to either null or not_null -- but not today, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No not for _LOAD_ATTR - this opcode is special, it means we dont know anything and it's only decideable at runtime.

}
}

op(_LOAD_ATTR_MODULE, (index/1, owner -- attr, null if (oparg & 1))) {
(void)index;
OUT_OF_SPACE_IF_NULL(null = sym_new_null(ctx));
Expand Down Expand Up @@ -515,11 +523,24 @@ dummy_func(void) {


op(_CHECK_FUNCTION_EXACT_ARGS, (func_version/2, callable, self_or_null, unused[oparg] -- callable, self_or_null, unused[oparg])) {
if (sym_is_const(callable) &&
sym_matches_type(callable, &PyFunction_Type) &&
(sym_is_null(self_or_null) || sym_is_not_null(self_or_null))) {
assert(PyFunction_Check(sym_get_const(callable)));
PyFunctionObject *func = (PyFunctionObject *)sym_get_const(callable);
if (func->func_version != func_version) {
goto hit_bottom;
}
PyCodeObject *code = (PyCodeObject *)func->func_code;
int argcount = oparg + sym_is_not_null(self_or_null);
if (code->co_argcount != argcount) {
goto hit_bottom;
}
REPLACE_OP(this_instr, _NOP, 0, 0);
}
if (!sym_set_type(callable, &PyFunction_Type)) {
goto hit_bottom;
}
(void)self_or_null;
(void)func_version;
}

op(_CHECK_CALL_BOUND_METHOD_EXACT_ARGS, (callable, null, unused[oparg] -- callable, null, unused[oparg])) {
Expand Down
Loading