From 080e145af4de4b60f869396d7414b795cb1ef3d2 Mon Sep 17 00:00:00 2001 From: Dorian Hoxha Date: Tue, 18 Aug 2026 13:07:44 +0200 Subject: [PATCH] Keep guards that prove later code unreachable Two TO_BOOL sites reading the same local can specialize on different types, so the JIT ends up guarding one value as both a list and None. The second guard can only fail, its output is Bottom, and the block is truncated to `guard; Unreachable`. Nothing consumed the list-ness, so GuardTypeRemoval dropped the GuardType; the None guard then succeeded and the compiled function hit its own ud2 (SIGILL). guardNeeded() now recomputes the output type of a Bottom-valued use with the relaxed operand type. If it is no longer Bottom, the guard is what makes the code unreachable, so keep it. Hit in SQLAlchemy's inspect_formatargspec under jit.auto(). Free-threaded builds only: on GIL builds emitGetLengthInt64 pins the list type with a UseType, which kept the guard alive by accident. --- cinderx/Jit/hir/guard_removal.cpp | 21 +++++++++++++ .../test_cinderx/test_jit_specialization.py | 31 +++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/cinderx/Jit/hir/guard_removal.cpp b/cinderx/Jit/hir/guard_removal.cpp index 0f3e7de89..cf849023b 100644 --- a/cinderx/Jit/hir/guard_removal.cpp +++ b/cinderx/Jit/hir/guard_removal.cpp @@ -52,6 +52,27 @@ bool guardNeeded(const RegUses& uses, Register* new_reg, Type relaxed_type) { worklist.emplace(passthrough_output, passthrough_type); } } + Register* output = instr->output(); + if (output != nullptr && output->isA(TBottom)) { + // This instruction can only fail, and the guard's narrower + // operand type is what proves it. The code after it has already + // been replaced with a trap, so widening the type would let the + // instruction succeed and fall into that trap. + Type relaxed_output = + outputType(*instr, [&](std::size_t operand_index) { + if (operand_index == i) { + return relaxed_type; + } + return instr->getOperand(operand_index)->type(); + }); + if (!(relaxed_output <= TBottom)) { + TRACE( + "'{}' kept alive by unreachable '{}'", + *new_reg->instr(), + *instr); + return true; + } + } OperandType expected_type = instr->getOperandType(i); // TASK(T106726658): We should be able to remove GuardTypes if we ever // add a matching constraint for non-Primitive types, and our diff --git a/cinderx/PythonLib/test_cinderx/test_jit_specialization.py b/cinderx/PythonLib/test_cinderx/test_jit_specialization.py index f06d21e4b..a52b14beb 100644 --- a/cinderx/PythonLib/test_cinderx/test_jit_specialization.py +++ b/cinderx/PythonLib/test_cinderx/test_jit_specialization.py @@ -462,6 +462,37 @@ def f(a: object) -> str: self.assertIn("TO_BOOL_NONE", opnames(f)) self.assertEqual(f(None), "n") + @passUnless(sys.version_info >= (3, 14), "TO_BOOL was added in Python 3.13") + def test_to_bool_list_then_none(self) -> None: + # Both TO_BOOL sites read `defaults`, but only calls with a non-empty + # `args` reach the second one, so they specialize on different types. + # The JIT ends up with contradictory guards, proves the code after the + # second one unreachable and traps there. The list guard is the only + # thing left that can send this call back to the interpreter. + def f(defaults: object, args: list[str]) -> list[str]: + first = 0 if defaults else -1 + out: list[str] = [] + for i, arg in enumerate(args): + if defaults and i >= first: + out.append("d") + out.append(arg) + return out + + def warm() -> None: + # An empty `args` keeps the loop body cold, so only the first + # TO_BOOL specializes here. + for _ in range(20): + f([1], []) + f(None, ["a"]) + + specialize(f, warm) + + ops = opnames(f) + self.assertIn("TO_BOOL_LIST", ops) + self.assertIn("TO_BOOL_NONE", ops) + self.assertEqual(f(None, ["a", "b"]), ["a", "b"]) + self.assertEqual(f([1], ["a", "b"]), ["d", "a", "d", "b"]) + @passUnless(sys.version_info >= (3, 14), "TO_BOOL was added in Python 3.13") def test_to_bool_str(self) -> None: def f(a: str) -> str: