GH-106008: Make implicit boolean conversions explicit by brandtbucher · Pull Request #106003 · python/cpython · GitHub

...and specialize them!

This adds a new TO_BOOL bytecode that prefixes all UNARY_NOT, POP_JUMP_IF_TRUE, and POP_JUMP_IF_FALSE instructions, which now require an exact boolean. We also use a spare bit in COMPARE_OP's oparg to indicate whether the result should be converted to bool (this saves a TO_BOOL for most branches, and is a no-op for all COMPARE_OP specializations).

"0% faster". Stats show a 93.5% hit rate for the new instructions.

📚 Documentation preview 📚: https://cpython-previews--106003.org.readthedocs.build/

Issue: Make implicit boolean conversions explicit #106008

A few minor comments, otherwise LGTM.

True, but the not the most useful comment for someone trying to understand the code 🙂

I think we decided it wasn't, as it reflects the underlying type instability when doing if x: as a stand in for if x is None.

Use value == &_Py_STR(empty). The semantics is value == "", not value is "".
In general I wouldn't use Py_Is except when you want the exact semantics of Python's x is y.

Hm, I mean, we are checking for the identity of the singleton string here. I'll just change it, though.

Hypothetically we could have more than one "" object. I don't think that " ".strip() is "" is part of the language spec, so Py_Is doesn't add any safety, just obfuscation.

We can also potentially have tagged ints, in which case Py_Is would need to become a lot more complex, but value == &_Py_STR(empty) would remain efficient.

See comment above.

This is so much more pleasing 🙂

Thanks, I tend to forget about the stats dict .

Note for possible future PR:

We could, at the cost of two bits in tp_flags avoid the version number and combine the ALWAYS_TRUE and NONE specializations.
We need a ALWAYS_TRUE_OR_FALSE and a IS_TRUE bit.
Classes that don't override __bool__ or __len__ and None would set the ALWAYS_TRUE_OR_FALSE bit. The IS_TRUE bit would be set to 0 for None, and to 1 for always true objects.

Rather than check the version number, check the ALWAYS_TRUE_OR_FALSE, then res = (tp_flags & IS_TRUE) ? Py_True : Py_False;

For abi4, we could add a per-object bit to handle immutable objects like ints and strings.

windows compiler warning here, implicit cast from Py_ssize_t to int

Merging is currently blocked on #106250.

Hey @brandtbucher, I have a question about this PR. In #106393 I had to change the code in POP_JUMP_IF_TRUE/FALSE from

JUMPBY(oparg * Py_IsFalse(cond));

to

if (Py_IsFalse(cond)) {
    JUMP_POP_DISPATCH(oparg, 1);  // Macro that wraps JUMPBY()
}

The reason is that the uop executor currently exits whenever it jumps, and your original code from this PR always jumps.

Did you (or @markshannon) have a reason to prefer the oparg * Py_IsFalse(cond) version over the conditional?

If there's no deep reason I'll keep it the way I coded it up; but if there is (maybe it came out faster in a micro-benchmark?) then I suppose I could fix it another way in the uop interpreter (e.g. only exiting if the jump offset is nonzero).

That the implementation of branches is itself branchless has a certain aesthetic appeal 🙂. TBH, that's the main reason.

The multiplication form will be quicker for unpredictable branches, and slower for predictable ones in the tier 1 interpreter.
I doubt it makes any difference in terms of overall performance, and will likely be irrelevant once the tier 2 interpreter does most of the work.

Summary: Implements what we need from [GH-106008](python/cpython#106003). Technically the included updates to `UNARY_NOT`, and `POP_JUMP_IF_FALSE|TRUE` are optimizations and not strictly needed. However, it really makes sense to to them. Reviewed By: DinoV Differential Revision: D80682012 fbshipit-source-id: 6150d52dddd28ef5c2e401ed17df7c7cad09074b

brandtbucher added 15 commits June 6, 2023 13:58

Make boolean conversions in branches explicit

69781b4

Specialize UNARY_NOT

5289c7f

Remove unused stats

7e5c9df

Make UNARY_NOT_BOOL a little bit slicker

27bb264

Replace UNARY_NOT with TO_BOOL

0463633

Catch up with main

d2932a1

Branchless branching

47ec38f

Add a "boolean conversion" bit to COMPARE_OP

a0bd53a

More stats for heap types

bab539b

Specialize TO_BOOL_ALWAYS_TRUE

757b1cc

Fix refleak and error handling

f9fa498

Catch up with main

0c14344

Cleanup

d314078

blurb add

9aa83b5

Docs

a76f11d

brandtbucher added performance Performance or resource usage interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Jun 23, 2023

brandtbucher requested a review from markshannon June 23, 2023 00:44

brandtbucher self-assigned this Jun 23, 2023

brandtbucher changed the title Make implicit boolean conversions explicit GH-106008: Make implicit boolean conversions explicit Jun 23, 2023

bedevere-bot mentioned this pull request Jun 23, 2023

Make implicit boolean conversions explicit #106008

Closed

brandtbucher marked this pull request as ready for review June 23, 2023 03:53

brandtbucher requested review from brettcannon, ericsnowcurrently, iritkatriel, ncoghlan and warsaw as code owners June 23, 2023 03:53

bedevere-bot added the awaiting core review label Jun 23, 2023

markshannon reviewed Jun 23, 2023

View reviewed changes

Clean up some commnts

c2d4a33

carljm reviewed Jun 23, 2023

View reviewed changes

TeamSpen210 reviewed Jun 24, 2023

View reviewed changes

Comment thread Python/flowgraph.c

tekknolagi mentioned this pull request Jun 27, 2023

Make boolean conversion explicit tekknolagi/skybison#516

Open

brettcannon removed their request for review June 27, 2023 22:54

Add cast

7638b76

Catch up with main

6b4a598

brandtbucher merged commit 7b2d94d into python:main Jun 29, 2023

bedevere-bot removed the awaiting core review label Jun 29, 2023

alexander-beedie mentioned this pull request May 18, 2024

feat(python): Start updating BytecodeParser for Python 3.13 pola-rs/polars#16304

Merged

+                      inst(TO_BOOL_STR, (unused/1, unused/2, value -- res)) {
+                          DEOPT_IF(!PyUnicode_CheckExact(value), TO_BOOL);
+                          STAT_INC(TO_BOOL, hit);
+                          if (Py_Is(value, &_Py_STR(empty))) {

                   err += add_stat_dict(stats, UNPACK_SEQUENCE, "unpack_sequence");
                   err += add_stat_dict(stats, FOR_ITER, "for_iter");
+                  err += add_stat_dict(stats, TO_BOOL, "to_bool");
+                  err += add_stat_dict(stats, SEND, "send");

+                      }
+                      inst(TO_BOOL_BOOL, (unused/1, unused/2, value -- value)) {
+                          // Coolest (and dumbest-named) specialization ever:

+                      inst(TO_BOOL_NONE, (unused/1, unused/2, value -- res)) {
+                          // This one is a bit weird, because we expect *some* failures...
+                          // it might be worth combining with TO_BOOL_ALWAYS_TRUE somehow:

+                      inst(TO_BOOL_ALWAYS_TRUE, (unused/1, version/2, value -- res)) {
+                          // This one is a bit weird, because we expect *some* failures...
+                          // it might be worth combining with TO_BOOL_NONE somehow:

+                      }
+                  }
+                  Py_DECREF(newconst);
+                  return index;

GH-106008: Make implicit boolean conversions explicit by brandtbucher · Pull Request #106003 · python/cpython · GitHub

Uh oh!

Uh oh!

GH-106008: Make implicit boolean conversions explicit - #106003

GH-106008: Make implicit boolean conversions explicit#106003brandtbucher merged 18 commits intopython:mainpython/cpython:mainfrom brandtbucher:to-boolbrandtbucher/cpython:to-boolCopy head branch name to clipboard

Conversation

brandtbucher commented Jun 23, 2023 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

markshannon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markshannon Jun 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markshannon commented Jun 23, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

brandtbucher commented Jun 29, 2023

Uh oh!

gvanrossum commented Jul 4, 2023

Uh oh!

markshannon commented Jul 4, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

GH-106008: Make implicit boolean conversions explicit#106003
brandtbucher merged 18 commits into
python:mainfrom
brandtbucher:to-bool

brandtbucher commented Jun 23, 2023 •
edited by bedevere-bot
Loading

markshannon Jun 26, 2023 •
edited
Loading