Skip to content

gh-115816: Improve internal symbols API in optimizer #116028

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
gvanrossum merged 15 commits into from
Feb 28, 2024
Merged

gh-115816: Improve internal symbols API in optimizer #116028

Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
9696b67
Reorganize flag bits
gvanrossum Feb 27, 2024
cd120a4
Fix top/bottom tests (bottom has all properties off)
gvanrossum Feb 27, 2024
ffdf029
Add _Py_uop_sym_set_non_null in various places
gvanrossum Feb 27, 2024
7d44eed
Rename arg of _Py_uop_sym_set_type to typ
gvanrossum Feb 27, 2024
ac83bb6
Explain members of Symbol type
gvanrossum Feb 27, 2024
c6c7c66
sym_set_type() may result in bottom
gvanrossum Feb 27, 2024
0cda7a8
Add more tests
gvanrossum Feb 28, 2024
5b0c862
Add sym_set_const and its tests
gvanrossum Feb 28, 2024
785bafb
Simplify sym_new signature
gvanrossum Feb 28, 2024
17a8d67
Incorporate assertions into sym_is_bottom()
gvanrossum Feb 28, 2024
bc39039
Fix set_bottom name; add sym_set_const macro
gvanrossum Feb 28, 2024
c355bc6
Simplification and decrefs from code review
gvanrossum Feb 28, 2024
5362360
Merge remote-tracking branch 'origin/main' into new-symbols
gvanrossum Feb 28, 2024
761d2b1
Declare val_42/43 earlier.
gvanrossum Feb 28, 2024
b0509ce
Fix bug -- if sym->typ is NULL, don't cry bottom
gvanrossum Feb 28, 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
Prev Previous commit
Fix bug -- if sym->typ is NULL, don't cry bottom
  • Loading branch information
@gvanrossum
gvanrossum committed Feb 28, 2024
commit b0509ce79405748556b78946a4c81f235dd91c03
2 changes: 1 addition & 1 deletion Python/optimizer_symbols.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ _Py_uop_sym_set_const(_Py_UopsSymbol *sym, PyObject *const_val)
return;
}
PyTypeObject *typ = Py_TYPE(const_val);
if (sym->typ != typ) {
if (sym->typ != NULL && sym->typ != typ) {
Copy link
Copy Markdown
Member

@Fidget-Spinner Fidget-Spinner Feb 28, 2024

Choose a reason for hiding this comment

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

Could be simplified to

Suggested change
if (sym->typ != NULL && sym->typ != typ) {
if (sym->typ != typ) {

as Py_TYPE(const_val) cannot be NULL.

Copy link
Copy Markdown
Member

@Fidget-Spinner Fidget-Spinner Feb 28, 2024

Choose a reason for hiding this comment

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

Oh I see this was a bug I introduced. Sorry.

Copy link
Copy Markdown
Member Author

@gvanrossum gvanrossum Feb 28, 2024

Choose a reason for hiding this comment

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

Turns out this "fix" was wrong -- we're not checking whether typ is NULL, we're checking whether sym->typ isn't NULL. If it's NULL, setting the constant should be allowed. Running the tests with PYTHONUOPSOPTIMIZE=1 revealed this. :-)

sym_set_bottom(sym);
return;
}
Expand Down