Skip to content

Commit c836d75

Browse files
yoneymeta-codesync[bot]
yoney
authored and
meta-codesync[bot]
committed
Optimize tuple subscripts in free-threaded builds
Summary: Enable the direct LoadArrayItem fast path for exact tuples, which are immutable and safe to borrow from in free-threaded builds. Reviewed By: mpage Differential Revision: D114727265 fbshipit-source-id: 5eea7f64c683032c189ea0d2015a0b651715de03
1 parent 1922df2 commit c836d75

4 files changed

Lines changed: 227 additions & 49 deletions

File tree

‎cinderx/Jit/hir/simplify.cpp‎

Lines changed: 32 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -901,7 +901,14 @@ Register* unboxAndCheckListOrTupleIndex(
901901
lhs->type());
902902
JIT_CHECK(
903903
rhs->isA(TLongExact), "rhs must be a TLongExact, not a {}", rhs->type());
904-
JIT_CHECK(!kFreeThreadedBuild, "only valid for the default build");
904+
// A free-threaded build must not hand out a borrowed reference to a list
905+
// slot; another thread can overwrite it at any time. Exact tuples are
906+
// immutable once published, so their items stay valid while the tuple is
907+
// alive.
908+
JIT_CHECK(
909+
!kFreeThreadedBuild || lhs->isA(TTupleExact),
910+
"only valid for exact tuples in a free-threaded build, not a {}",
911+
lhs->type());
905912

906913
if (rhs->type().hasObjectSpec()) {
907914
return checkConstantListOrTupleIndex(env, lhs, rhs);
@@ -982,33 +989,32 @@ Register* simplifySubscript(Env& env, const BinaryOp* instr) {
982989
}
983990
}
984991

985-
if constexpr (kFreeThreadedBuild) {
986-
// In free-threaded builds we can't use the borrowed LoadArrayItem path
987-
// for lists because another thread may overwrite the slot at any time.
988-
// With an exact list, we can still skip PyObject_GetItem's generic
989-
// dispatch by emitting ListSubscr, which returns an owned reference
990-
// via PyList_GetItemRef.
991-
if (lhs->isA(TListExact)) {
992-
return env.emit<ListSubscr>(lhs, rhs, *instr->frameState());
992+
// In free-threaded builds we can't use the borrowed LoadArrayItem path for
993+
// lists because another thread may overwrite the slot at any time. With an
994+
// exact list, we can still skip PyObject_GetItem's generic dispatch by
995+
// emitting ListSubscr, which returns an owned reference via
996+
// PyList_GetItemRef. Exact tuples are immutable once published, so their
997+
// items stay alive as long as the tuple does and keep the direct path.
998+
if (kFreeThreadedBuild && lhs->isA(TListExact)) {
999+
return env.emit<ListSubscr>(lhs, rhs, *instr->frameState());
1000+
}
1001+
1002+
if (lhs->isA(TListExact) || lhs->isA(TTupleExact)) {
1003+
Register* adjusted_idx =
1004+
unboxAndCheckListOrTupleIndex(env, instr, lhs, rhs);
1005+
if (adjusted_idx == nullptr) {
1006+
return nullptr;
9931007
}
994-
} else {
995-
if (lhs->isA(TListExact) || lhs->isA(TTupleExact)) {
996-
Register* adjusted_idx =
997-
unboxAndCheckListOrTupleIndex(env, instr, lhs, rhs);
998-
if (adjusted_idx == nullptr) {
999-
return nullptr;
1000-
}
1001-
Py_ssize_t offset = offsetof(PyTupleObject, ob_item);
1002-
Register* array = lhs;
1003-
// Lists carry a nested array of ob_item whereas tuples are variable-sized
1004-
// structs.
1005-
if (lhs->isA(TListExact)) {
1006-
array = env.emit<LoadField>(
1007-
lhs, "ob_item", offsetof(PyListObject, ob_item), TCPtr);
1008-
offset = 0;
1009-
}
1010-
return env.emit<LoadArrayItem>(array, adjusted_idx, lhs, offset, TObject);
1008+
Py_ssize_t offset = offsetof(PyTupleObject, ob_item);
1009+
Register* array = lhs;
1010+
// Lists carry a nested array of ob_item whereas tuples are variable-sized
1011+
// structs.
1012+
if (lhs->isA(TListExact)) {
1013+
array = env.emit<LoadField>(
1014+
lhs, "ob_item", offsetof(PyListObject, ob_item), TCPtr);
1015+
offset = 0;
10111016
}
1017+
return env.emit<LoadArrayItem>(array, adjusted_idx, lhs, offset, TObject);
10121018
}
10131019

10141020
// Unicode subscript.

‎cinderx/RuntimeTests/hir_tests/all_passes_static_test.txt‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,4 +498,132 @@ fun jittestmodule:test {
498498
Return v44
499499
}
500500
}
501+
--- Expected 3.14t ---
502+
fun jittestmodule:test {
503+
bb 0 {
504+
v22:TupleExact = LoadArg<0; "x", TupleExact>
505+
LoadFrame
506+
v25:Nullptr = LoadConst<Nullptr>
507+
v24:TupleExact = TagIfDeferred v22
508+
AtQuiescentState
509+
UpdatePrevInstr<idx:0 line_no:0: no parent>
510+
v26:CInt64 = LoadEvalBreaker
511+
CondBranch<5, 4> v26
512+
}
513+
514+
bb 5 (preds 0) {
515+
v48:CInt32 = RunPeriodicTasks {
516+
LiveValues<2> b:v24 unc:v25
517+
FrameState {
518+
CurInstrOffset 0
519+
Locals<3> v24 v25 v25
520+
}
521+
}
522+
Branch<4>
523+
}
524+
525+
bb 4 (preds 0, 5) {
526+
v27:CInt64[0] = LoadConst<CInt64[0]>
527+
Branch<6>
528+
}
529+
530+
bb 6 (preds 4, 10) {
531+
v30:CInt64 = Phi<4, 10> v27 v39
532+
v31:OptObject = Phi<4, 10> v25 v57
533+
AtQuiescentState
534+
v32:CInt64 = LoadEvalBreaker
535+
CondBranch<7, 1> v32
536+
}
537+
538+
bb 7 (preds 6) {
539+
v47:CInt32 = RunPeriodicTasks {
540+
LiveValues<3> b:v24 s:v30 o:v31
541+
FrameState {
542+
CurInstrOffset 12
543+
Locals<3> v24 v30 v31
544+
Stack<1> v24
545+
}
546+
}
547+
Branch<1>
548+
}
549+
550+
bb 1 (preds 6, 7) {
551+
v33:CInt64 = LoadField<ob_size@32, CInt64, borrowed> v24
552+
v34:CBool = PrimitiveCompare<GreaterThan> v33 v30
553+
CondBranch<2, 3> v34
554+
}
555+
556+
bb 2 (preds 1) {
557+
v59:CBool = IsCompactLong v30
558+
Guard v59 {
559+
LiveValues<4> b:v24 s:v30 o:v31 bool:v59
560+
FrameState {
561+
CurInstrOffset 34
562+
Locals<3> v24 v30 v31
563+
Stack<2> v24 v24
564+
}
565+
}
566+
v51:CInt64 = LoadField<ob_size@32, CInt64, borrowed> v24
567+
v52:CInt64[0] = LoadConst<CInt64[0]>
568+
v53:CBool = PrimitiveCompare<LessThan> v30 v52
569+
CondBranch<8, 9> v53
570+
}
571+
572+
bb 8 (preds 2) {
573+
v54:CInt64 = IntBinaryOp<Add> v30 v51
574+
Branch<10>
575+
}
576+
577+
bb 9 (preds 2) {
578+
Branch<10>
579+
}
580+
581+
bb 10 (preds 8, 9) {
582+
v55:CInt64 = Phi<8, 9> v54 v30
583+
v56:CBool = PrimitiveCompare<LessThanUnsigned> v55 v51
584+
Guard v56 {
585+
LiveValues<5> b:v24 s:v30 o:v31 s:v55 bool:v56
586+
FrameState {
587+
CurInstrOffset 34
588+
Locals<3> v24 v30 v31
589+
Stack<2> v24 v24
590+
}
591+
}
592+
v57:Object = LoadArrayItem<Offset[48]> v24 v55 v24
593+
v38:CInt64[1] = LoadConst<CInt64[1]>
594+
v39:CInt64 = IntBinaryOp<Add> v30 v38
595+
v42:OptObject = LoadGlobalCached<0; "print">
596+
v43:MortalObjectUser[builtin_function_or_method:print:0xdeadbeef] = GuardIs<0xdeadbeef> v42 {
597+
Descr 'LOAD_GLOBAL: print'
598+
LiveValues<6> b:v24 s:v30 o:v31 s:v39 b:v42 b:v57
599+
FrameState {
600+
CurInstrOffset 66
601+
Locals<3> v24 v30 v31
602+
Stack<3> v24 v57 v39
603+
}
604+
}
605+
Incref v57
606+
v62:OptObject = MaterializeRef v31
607+
XDecref v62
608+
v58:Object = VectorCall<1> v43 v57 {
609+
LiveValues<4> b:v24 s:v39 b:v43 o:v57
610+
FrameState {
611+
CurInstrOffset 84
612+
Locals<3> v24 v39 v57
613+
Stack<1> v24
614+
}
615+
}
616+
v63:Object = MaterializeRef v58
617+
Decref v63
618+
Branch<6>
619+
}
620+
621+
bb 3 (preds 1) {
622+
v60:OptObject = MaterializeRef v31
623+
XDecref v60
624+
v46:ImmortalNoneType = LoadConst<ImmortalNoneType>
625+
v61:ImmortalNoneType = MaterializeRef v46
626+
Return v61
627+
}
628+
}
501629
--- End ---

‎cinderx/RuntimeTests/hir_tests/simplify_static_test.txt‎

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2124,9 +2124,9 @@ fun jittestmodule:test {
21242124
Branch<6>
21252125
}
21262126

2127-
bb 6 (preds 2, 4) {
2128-
v30:CInt64 = Phi<2, 4> v39 v27
2129-
v31:OptObject = Phi<2, 4> v37 v25
2127+
bb 6 (preds 4, 10) {
2128+
v30:CInt64 = Phi<4, 10> v27 v39
2129+
v31:OptObject = Phi<4, 10> v25 v57
21302130
AtQuiescentState
21312131
v32:CInt64 = LoadEvalBreaker
21322132
CondBranch<7, 1> v32
@@ -2162,13 +2162,34 @@ fun jittestmodule:test {
21622162
Stack<2> v24 v24
21632163
}
21642164
}
2165-
v37:Object = BinaryOp<Subscript> v24 v36 {
2166-
FrameState {
2167-
CurInstrOffset 42
2168-
Locals<3> v24 v30 v31
2169-
Stack<1> v24
2170-
}
2165+
UseType<TupleExact> v24
2166+
UseType<LongExact> v36
2167+
v59:CBool = IsCompactLong v30
2168+
Guard v59 {
2169+
}
2170+
UseType<TupleExact> v24
2171+
v51:CInt64 = LoadField<ob_size@32, CInt64, borrowed> v24
2172+
v52:CInt64[0] = LoadConst<CInt64[0]>
2173+
v53:CBool = PrimitiveCompare<LessThan> v30 v52
2174+
CondBranch<8, 9> v53
2175+
}
2176+
2177+
bb 8 (preds 2) {
2178+
v54:CInt64 = IntBinaryOp<Add> v30 v51
2179+
Branch<10>
2180+
}
2181+
2182+
bb 9 (preds 2) {
2183+
Branch<10>
2184+
}
2185+
2186+
bb 10 (preds 8, 9) {
2187+
v55:CInt64 = Phi<8, 9> v54 v30
2188+
Snapshot
2189+
v56:CBool = PrimitiveCompare<LessThanUnsigned> v55 v51
2190+
Guard v56 {
21712191
}
2192+
v57:Object = LoadArrayItem<Offset[48]> v24 v55 v24
21722193
Snapshot
21732194
v38:CInt64[1] = LoadConst<CInt64[1]>
21742195
v39:CInt64 = IntBinaryOp<Add> v30 v38
@@ -2179,10 +2200,10 @@ fun jittestmodule:test {
21792200
}
21802201
v44:Nullptr = LoadConst<Nullptr>
21812202
Snapshot
2182-
v49:Object = VectorCall<1> v43 v37 {
2203+
v58:Object = VectorCall<1> v43 v57 {
21832204
FrameState {
21842205
CurInstrOffset 84
2185-
Locals<3> v24 v39 v37
2206+
Locals<3> v24 v39 v57
21862207
Stack<1> v24
21872208
}
21882209
}

‎cinderx/RuntimeTests/hir_tests/simplify_test.txt‎

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2145,12 +2145,36 @@ fun test {
21452145
v2:TupleExact = RefineType<TupleExact> v0
21462146
v3:LongExact = RefineType<LongExact> v1
21472147
Snapshot
2148-
v4:Object = BinaryOp<Subscript> v2 v3 {
2149-
FrameState {
2150-
CurInstrOffset -2
2151-
}
2148+
UseType<TupleExact> v2
2149+
UseType<LongExact> v3
2150+
v5:CBool = IsCompactLong v3
2151+
Guard v5 {
21522152
}
2153-
Return v4
2153+
v6:CInt64 = CompactLongUnbox v3
2154+
UseType<TupleExact> v2
2155+
v7:CInt64 = LoadField<ob_size@32, CInt64, borrowed> v2
2156+
v8:CInt64[0] = LoadConst<CInt64[0]>
2157+
v9:CBool = PrimitiveCompare<LessThan> v6 v8
2158+
CondBranch<1, 2> v9
2159+
}
2160+
2161+
bb 1 (preds 0) {
2162+
v10:CInt64 = IntBinaryOp<Add> v6 v7
2163+
Branch<3>
2164+
}
2165+
2166+
bb 2 (preds 0) {
2167+
Branch<3>
2168+
}
2169+
2170+
bb 3 (preds 1, 2) {
2171+
v11:CInt64 = Phi<1, 2> v10 v6
2172+
Snapshot
2173+
v12:CBool = PrimitiveCompare<LessThanUnsigned> v11 v7
2174+
Guard v12 {
2175+
}
2176+
v13:Object = LoadArrayItem<Offset[48]> v2 v11 v2
2177+
Return v13
21542178
}
21552179
}
21562180
--- Test Name ---
@@ -13046,14 +13070,13 @@ fun jittestmodule:test {
1304613070
Snapshot
1304713071
v24:ImmortalLongExact[1] = LoadConst<ImmortalLongExact[1]>
1304813072
Snapshot
13049-
v25:Object = BinaryOp<Subscript> v22 v24 {
13050-
FrameState {
13051-
CurInstrOffset 14
13052-
Locals<4> v17 v18 v19 v22
13053-
}
13054-
}
13073+
UseType<MortalTupleExact> v22
13074+
UseType<ImmortalLongExact[1]> v24
13075+
v27:CInt64[1] = LoadConst<CInt64[1]>
13076+
UseType<TupleExact> v22
13077+
UseType<CInt64[1]> v27
1305513078
Snapshot
13056-
Return v25
13079+
Return v18
1305713080
}
1305813081
}
1305913082
--- Test Name ---

0 commit comments

Comments
 (0)