JIT optimizer: `!sym_is_null(obj)` is not same as `sym_is_not_null(obj)`

[Fidget-Spinner]

Bug report

Bug description:

The problem with the above pattern is that obj has three states in terms of nullness, NULL/not-NULL/UKNOWN, so !not-null = UKNOWN/NULL which is not what the optimization is usually expecting (the unknown part).

CPython versions tested on:

CPython main branch

Operating systems tested on:

No response

Linked PRs

• gh-148078: Fix uses of sym_is_not_null in JIT optimizer #148079

[markshannon]

Don't forget the else clauses.

    if (sym_is_not_null(x)) {
         // Do something
    }
    else {
         // Do something else
    }

should be

    if (sym_is_null(x)) {
         // Do something
    }
    else  if (sym_is_not_null(x)) {
         // Do something else
    }
    else {
        break;
    }

[Fidget-Spinner]

@markshannon I looked through the current code. All the uses of that pattern are safe as they all follow this form or similar:

if (sym_is_not_null(x) || sym_is_null(x)) {
    if (sym_is_not_null(x)) {
         // Do something
    }
    else {
         // Do something else
    }
}

This pattern is safe, it's only if we don't check for both of them at the start (or at the end, like you pointed out) which makes it unsafe.

The other pattern that is safe is if the opcode explictly needs a NULL, then:

    if (sym_is_null(x)) {
         // Do something
    }
    else {
         // Do something else
    }

is also safe.

[markshannon]

This line is incorrect: https://github.com/python/cpython/blob/main/Python/optimizer_bytecodes.c#L1328
as there is an implicit else leaving total_args set to oparg.
If we don't know whether self_or_null is NULL, we cannot know total_args.

https://github.com/python/cpython/blob/main/Python/optimizer_bytecodes.c#L1336 is also wrong, for much the same reasons.

There are also other examples.

[Fidget-Spinner]

This line is incorrect: https://github.com/python/cpython/blob/main/Python/optimizer_bytecodes.c#L1328 as there is an implicit else leaving total_args set to oparg. If we don't know whether self_or_null is NULL, we cannot know total_args.

That line is correct. Please see the preceding check at https://github.com/python/cpython/blob/main/Python/optimizer_bytecodes.c#L1326

The check narrows the value to NOT-NULL|NULL, removing the unknown.

https://github.com/python/cpython/blob/main/Python/optimizer_bytecodes.c#L1336 is also wrong, for much the same reasons.

That line is correct for much the same reasons, see above https://github.com/python/cpython/blob/main/Python/optimizer_bytecodes.c#L1326