Skip to content

Commit ad70feb

Browse files
eytan-starkwareeytan-starkware
authored
(bug fix): prune usage after change promotion in loop usage finalization (#10201)
1 parent 8ca3ee7 commit ad70feb

2 files changed

Lines changed: 285 additions & 16 deletions

File tree

‎crates/cairo-lang-semantic/src/usage/mod.rs‎

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -79,21 +79,8 @@ impl<'db> Usage<'db> {
7979
/// Removes usage that was introduced current block and usage that is already covered
8080
/// by containing variables.
8181
pub fn finalize_as_scope(&mut self) {
82-
// Prune introductions from usages.
83-
for member_path in prune_and_get_candidates(&mut self.usage, |k| {
84-
self.introductions.contains(&k.base_var())
85-
}) {
86-
// Prune usages that are members of other usages.
87-
let mut current_path = &member_path;
88-
while let MemberPath::Member { parent, .. } = current_path {
89-
current_path = parent.as_ref();
90-
if self.usage.contains_key(current_path) {
91-
self.usage.swap_remove(&member_path);
92-
break;
93-
}
94-
}
95-
}
96-
// Prune usages and introdictions from snap_usage.
82+
// Prune usages and introdictions from snap_usage. Runs before the `changes` pass so that
83+
// the promotion below always promotes the top-most snapshotted ancestor.
9784
for member_path in prune_and_get_candidates(&mut self.snap_usage, |k| {
9885
self.usage.contains_key(k) || self.introductions.contains(&k.base_var())
9986
}) {
@@ -114,7 +101,9 @@ impl<'db> Usage<'db> {
114101
self.introductions.contains(&k.base_var())
115102
}) {
116103
// Prune changes that are members of other changes.
117-
// Also if a child is changed and its parent is used, then we change the parent.
104+
// Also if a child is changed and an ancestor is used as a snapshot, promote the
105+
// ancestor into `usage`/`changes` so the whole ancestor is read and written back,
106+
// instead of returning the child of a stale snapshot.
118107
// TODO(TomerStarkware): Deconstruct the parent, and snap_use other members.
119108
let mut current_path = &member_path;
120109
while let MemberPath::Member { parent, .. } = current_path {
@@ -133,6 +122,21 @@ impl<'db> Usage<'db> {
133122
}
134123
}
135124
}
125+
// Prune introductions from usages. Runs after the `changes` pass so that members of an
126+
// ancestor promoted above are pruned as well.
127+
for member_path in prune_and_get_candidates(&mut self.usage, |k| {
128+
self.introductions.contains(&k.base_var())
129+
}) {
130+
// Prune usages that are members of other usages.
131+
let mut current_path = &member_path;
132+
while let MemberPath::Member { parent, .. } = current_path {
133+
current_path = parent.as_ref();
134+
if self.usage.contains_key(current_path) {
135+
self.usage.swap_remove(&member_path);
136+
break;
137+
}
138+
}
139+
}
136140
}
137141
}
138142

‎tests/e2e_test_data/libfuncs/casm_run_sanity‎

Lines changed: 265 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,3 +448,268 @@ test::fib@F0([0]: RangeCheck, [1]: GasBuiltin, [2]: felt252) -> (RangeCheck, Gas
448448
test::fib[116-219]@F1([0]: RangeCheck, [1]: GasBuiltin, [2]: felt252, [3]: felt252, [4]: felt252, [5]: felt252) -> (RangeCheck, GasBuiltin, core::panics::PanicResult::<(core::felt252, core::felt252, core::felt252, ())>);
449449
core::panic_with_const_felt252::<375233589013918064796019>@F2() -> (Tuple<core::panics::Panic, Array<felt252>>);
450450
core::panic_with_felt252@F3([0]: felt252) -> (Tuple<core::panics::Panic, Array<felt252>>);
451+
452+
//! > ==========================================================================
453+
454+
//! > Loop that reads an ancestor as a snapshot (@a.b) while also mutating a nested field (a.b.c)
455+
456+
//! > must preserve the mutation. When `finalize_as_scope` promotes the snapped ancestor into
457+
458+
//! > usage/changes, the later usage-pruning pass subsumes its descendants so the loop reads and writes back the whole
459+
460+
//! > ancestor. For x=2 the result is 1*2*2=4 (0x4).
461+
462+
//! > test_runner_name
463+
SmallE2ETestRunner(test_data_function: loop_snap_mut, test_data_input: 2, test_data_output: 4)
464+
465+
//! > cairo_code
466+
#[derive(Drop)]
467+
struct Inner {
468+
c: felt252,
469+
}
470+
#[derive(Drop)]
471+
struct Outer {
472+
b: Inner,
473+
}
474+
475+
fn loop_snap_mut(x: felt252) -> felt252 {
476+
let mut a = Outer { b: Inner { c: 1 } };
477+
let mut i: felt252 = 0;
478+
loop {
479+
if i == x {
480+
break;
481+
}
482+
let _snap = @a.b;
483+
a.b.c = a.b.c * 2;
484+
i = i + 1;
485+
}
486+
a.b.c
487+
}
488+
489+
//! > casm
490+
[ap + 0] = [fp + -5], ap++;
491+
[ap + 0] = [fp + -4], ap++;
492+
[ap + 0] = 1, ap++;
493+
[ap + 0] = 0, ap++;
494+
[ap + 0] = [fp + -3], ap++;
495+
call rel 19;
496+
jmp rel 10 if [ap + -3] != 0;
497+
[ap + 0] = [ap + -5], ap++;
498+
[ap + 0] = [ap + -5], ap++;
499+
[ap + 0] = 0, ap++;
500+
[ap + 0] = 0, ap++;
501+
[ap + 0] = [ap + -6], ap++;
502+
ret;
503+
[ap + 0] = [ap + -5], ap++;
504+
[ap + 0] = [ap + -5], ap++;
505+
[ap + 0] = 1, ap++;
506+
[ap + 0] = [ap + -5], ap++;
507+
[ap + 0] = [ap + -5], ap++;
508+
ret;
509+
%{ memory[ap + 0] = 1270 <= memory[fp + -6] %}
510+
jmp rel 7 if [ap + 0] != 0, ap++;
511+
[ap + 0] = [fp + -6] + 340282366920938463463374607431768210186, ap++;
512+
[ap + -1] = [[fp + -7] + 0];
513+
jmp rel 28;
514+
[fp + -6] = [ap + 0] + 1270, ap++;
515+
[ap + -1] = [[fp + -7] + 0];
516+
[fp + -4] = [ap + 0] + [fp + -3], ap++;
517+
jmp rel 11 if [ap + -1] != 0;
518+
[ap + 0] = [fp + -7] + 1, ap++;
519+
[ap + 0] = [ap + -3] + 2070, ap++;
520+
[ap + 0] = 0, ap++;
521+
[ap + 0] = [fp + -5], ap++;
522+
[ap + 0] = [fp + -4], ap++;
523+
ret;
524+
[ap + 0] = [fp + -7] + 1, ap++;
525+
[ap + 0] = [ap + -3], ap++;
526+
[ap + 0] = [fp + -5] * 2, ap++;
527+
[ap + 0] = [fp + -4] + 1, ap++;
528+
[ap + 0] = [fp + -3], ap++;
529+
call rel -30;
530+
ret;
531+
call rel 10;
532+
[ap + 0] = [fp + -7] + 1, ap++;
533+
[ap + 0] = [fp + -6], ap++;
534+
[ap + 0] = 1, ap++;
535+
[ap + 0] = [ap + -5], ap++;
536+
[ap + 0] = [ap + -5], ap++;
537+
ret;
538+
[ap + 0] = 375233589013918064796019, ap++;
539+
call rel 3;
540+
ret;
541+
%{ memory[ap + 0] = segments.add() %}
542+
ap += 1;
543+
[fp + -3] = [[ap + -1] + 0];
544+
[ap + 0] = [ap + -1], ap++;
545+
[ap + 0] = [ap + -2] + 1, ap++;
546+
ret;
547+
548+
//! > function_costs
549+
test::loop_snap_mut: SmallOrderedMap({Const: 3170})
550+
test::loop_snap_mut[116-254]: SmallOrderedMap({Const: 1870})
551+
core::panic_with_const_felt252::<375233589013918064796019>: SmallOrderedMap({Const: 700})
552+
core::panic_with_felt252: SmallOrderedMap({Const: 400})
553+
554+
//! > sierra_code
555+
type felt252 = felt252 [storable: true, drop: true, dup: true, zero_sized: false];
556+
type core::panics::Panic = Struct<ut@core::panics::Panic> [storable: true, drop: true, dup: true, zero_sized: true];
557+
type Array<felt252> = Array<felt252> [storable: true, drop: true, dup: false, zero_sized: false];
558+
type Const<felt252, 375233589013918064796019> = Const<felt252, 375233589013918064796019> [storable: false, drop: false, dup: false, zero_sized: false];
559+
type Const<felt252, 2> = Const<felt252, 2> [storable: false, drop: false, dup: false, zero_sized: false];
560+
type NonZero<felt252> = NonZero<felt252> [storable: true, drop: true, dup: true, zero_sized: false];
561+
type Tuple<felt252> = Struct<ut@Tuple, felt252> [storable: true, drop: true, dup: true, zero_sized: false];
562+
type Tuple<core::panics::Panic, Array<felt252>> = Struct<ut@Tuple, core::panics::Panic, Array<felt252>> [storable: true, drop: true, dup: false, zero_sized: false];
563+
type core::panics::PanicResult::<(core::felt252,)> = Enum<ut@core::panics::PanicResult::<(core::felt252,)>, Tuple<felt252>, Tuple<core::panics::Panic, Array<felt252>>> [storable: true, drop: true, dup: false, zero_sized: false];
564+
type Unit = Struct<ut@Tuple> [storable: true, drop: true, dup: true, zero_sized: true];
565+
type test::Inner = Struct<ut@test::Inner, felt252> [storable: true, drop: true, dup: true, zero_sized: false];
566+
type Tuple<test::Inner, felt252, Unit> = Struct<ut@Tuple, test::Inner, felt252, Unit> [storable: true, drop: true, dup: true, zero_sized: false];
567+
type core::panics::PanicResult::<(test::Inner, core::felt252, ())> = Enum<ut@core::panics::PanicResult::<(test::Inner, core::felt252, ())>, Tuple<test::Inner, felt252, Unit>, Tuple<core::panics::Panic, Array<felt252>>> [storable: true, drop: true, dup: false, zero_sized: false];
568+
type GasBuiltin = GasBuiltin [storable: true, drop: false, dup: false, zero_sized: false];
569+
type RangeCheck = RangeCheck [storable: true, drop: false, dup: false, zero_sized: false];
570+
type Const<felt252, 0> = Const<felt252, 0> [storable: false, drop: false, dup: false, zero_sized: false];
571+
type Const<felt252, 1> = Const<felt252, 1> [storable: false, drop: false, dup: false, zero_sized: false];
572+
573+
libfunc disable_ap_tracking = disable_ap_tracking;
574+
libfunc const_as_immediate<Const<felt252, 1>> = const_as_immediate<Const<felt252, 1>>;
575+
libfunc const_as_immediate<Const<felt252, 0>> = const_as_immediate<Const<felt252, 0>>;
576+
libfunc snapshot_take<felt252> = snapshot_take<felt252>;
577+
libfunc drop<felt252> = drop<felt252>;
578+
libfunc struct_construct<test::Inner> = struct_construct<test::Inner>;
579+
libfunc store_temp<RangeCheck> = store_temp<RangeCheck>;
580+
libfunc store_temp<GasBuiltin> = store_temp<GasBuiltin>;
581+
libfunc store_temp<test::Inner> = store_temp<test::Inner>;
582+
libfunc store_temp<felt252> = store_temp<felt252>;
583+
libfunc function_call<user@test::loop_snap_mut[116-254]> = function_call<user@test::loop_snap_mut[116-254]>;
584+
libfunc enum_match<core::panics::PanicResult::<(test::Inner, core::felt252, ())>> = enum_match<core::panics::PanicResult::<(test::Inner, core::felt252, ())>>;
585+
libfunc branch_align = branch_align;
586+
libfunc redeposit_gas = redeposit_gas;
587+
libfunc struct_deconstruct<Tuple<test::Inner, felt252, Unit>> = struct_deconstruct<Tuple<test::Inner, felt252, Unit>>;
588+
libfunc drop<Unit> = drop<Unit>;
589+
libfunc struct_deconstruct<test::Inner> = struct_deconstruct<test::Inner>;
590+
libfunc struct_construct<Tuple<felt252>> = struct_construct<Tuple<felt252>>;
591+
libfunc enum_init<core::panics::PanicResult::<(core::felt252,)>, 0> = enum_init<core::panics::PanicResult::<(core::felt252,)>, 0>;
592+
libfunc store_temp<core::panics::PanicResult::<(core::felt252,)>> = store_temp<core::panics::PanicResult::<(core::felt252,)>>;
593+
libfunc enum_init<core::panics::PanicResult::<(core::felt252,)>, 1> = enum_init<core::panics::PanicResult::<(core::felt252,)>, 1>;
594+
libfunc withdraw_gas = withdraw_gas;
595+
libfunc dup<felt252> = dup<felt252>;
596+
libfunc rename<felt252> = rename<felt252>;
597+
libfunc felt252_sub = felt252_sub;
598+
libfunc felt252_is_zero = felt252_is_zero;
599+
libfunc struct_construct<Unit> = struct_construct<Unit>;
600+
libfunc struct_construct<Tuple<test::Inner, felt252, Unit>> = struct_construct<Tuple<test::Inner, felt252, Unit>>;
601+
libfunc enum_init<core::panics::PanicResult::<(test::Inner, core::felt252, ())>, 0> = enum_init<core::panics::PanicResult::<(test::Inner, core::felt252, ())>, 0>;
602+
libfunc store_temp<core::panics::PanicResult::<(test::Inner, core::felt252, ())>> = store_temp<core::panics::PanicResult::<(test::Inner, core::felt252, ())>>;
603+
libfunc drop<NonZero<felt252>> = drop<NonZero<felt252>>;
604+
libfunc const_as_immediate<Const<felt252, 2>> = const_as_immediate<Const<felt252, 2>>;
605+
libfunc felt252_mul = felt252_mul;
606+
libfunc felt252_add = felt252_add;
607+
libfunc drop<test::Inner> = drop<test::Inner>;
608+
libfunc function_call<user@core::panic_with_const_felt252::<375233589013918064796019>> = function_call<user@core::panic_with_const_felt252::<375233589013918064796019>>;
609+
libfunc enum_init<core::panics::PanicResult::<(test::Inner, core::felt252, ())>, 1> = enum_init<core::panics::PanicResult::<(test::Inner, core::felt252, ())>, 1>;
610+
libfunc const_as_immediate<Const<felt252, 375233589013918064796019>> = const_as_immediate<Const<felt252, 375233589013918064796019>>;
611+
libfunc function_call<user@core::panic_with_felt252> = function_call<user@core::panic_with_felt252>;
612+
libfunc array_new<felt252> = array_new<felt252>;
613+
libfunc array_append<felt252> = array_append<felt252>;
614+
libfunc struct_construct<core::panics::Panic> = struct_construct<core::panics::Panic>;
615+
libfunc struct_construct<Tuple<core::panics::Panic, Array<felt252>>> = struct_construct<Tuple<core::panics::Panic, Array<felt252>>>;
616+
libfunc store_temp<Tuple<core::panics::Panic, Array<felt252>>> = store_temp<Tuple<core::panics::Panic, Array<felt252>>>;
617+
618+
F0:
619+
disable_ap_tracking() -> ();
620+
const_as_immediate<Const<felt252, 1>>() -> ([3]);
621+
const_as_immediate<Const<felt252, 0>>() -> ([4]);
622+
snapshot_take<felt252>([2]) -> ([5], [6]);
623+
drop<felt252>([5]) -> ();
624+
struct_construct<test::Inner>([3]) -> ([7]);
625+
store_temp<RangeCheck>([0]) -> ([0]);
626+
store_temp<GasBuiltin>([1]) -> ([1]);
627+
store_temp<test::Inner>([7]) -> ([7]);
628+
store_temp<felt252>([4]) -> ([4]);
629+
store_temp<felt252>([6]) -> ([6]);
630+
function_call<user@test::loop_snap_mut[116-254]>([0], [1], [7], [4], [6]) -> ([8], [9], [10]);
631+
enum_match<core::panics::PanicResult::<(test::Inner, core::felt252, ())>>([10]) { fallthrough([11]) F0_B0([12]) };
632+
branch_align() -> ();
633+
redeposit_gas([9]) -> ([13]);
634+
struct_deconstruct<Tuple<test::Inner, felt252, Unit>>([11]) -> ([14], [15], [16]);
635+
drop<felt252>([15]) -> ();
636+
drop<Unit>([16]) -> ();
637+
struct_deconstruct<test::Inner>([14]) -> ([17]);
638+
struct_construct<Tuple<felt252>>([17]) -> ([18]);
639+
enum_init<core::panics::PanicResult::<(core::felt252,)>, 0>([18]) -> ([19]);
640+
store_temp<RangeCheck>([8]) -> ([8]);
641+
store_temp<GasBuiltin>([13]) -> ([13]);
642+
store_temp<core::panics::PanicResult::<(core::felt252,)>>([19]) -> ([19]);
643+
return([8], [13], [19]);
644+
F0_B0:
645+
branch_align() -> ();
646+
enum_init<core::panics::PanicResult::<(core::felt252,)>, 1>([12]) -> ([20]);
647+
store_temp<RangeCheck>([8]) -> ([8]);
648+
store_temp<GasBuiltin>([9]) -> ([9]);
649+
store_temp<core::panics::PanicResult::<(core::felt252,)>>([20]) -> ([20]);
650+
return([8], [9], [20]);
651+
F1:
652+
disable_ap_tracking() -> ();
653+
withdraw_gas([0], [1]) { fallthrough([5], [6]) F1_B1([7], [8]) };
654+
branch_align() -> ();
655+
dup<felt252>([4]) -> ([4], [9]);
656+
rename<felt252>([9]) -> ([10]);
657+
dup<felt252>([3]) -> ([3], [11]);
658+
felt252_sub([11], [10]) -> ([12]);
659+
store_temp<felt252>([12]) -> ([12]);
660+
felt252_is_zero([12]) { fallthrough() F1_B0([13]) };
661+
branch_align() -> ();
662+
drop<felt252>([4]) -> ();
663+
redeposit_gas([6]) -> ([14]);
664+
struct_construct<Unit>() -> ([15]);
665+
struct_construct<Tuple<test::Inner, felt252, Unit>>([2], [3], [15]) -> ([16]);
666+
enum_init<core::panics::PanicResult::<(test::Inner, core::felt252, ())>, 0>([16]) -> ([17]);
667+
store_temp<RangeCheck>([5]) -> ([5]);
668+
store_temp<GasBuiltin>([14]) -> ([14]);
669+
store_temp<core::panics::PanicResult::<(test::Inner, core::felt252, ())>>([17]) -> ([17]);
670+
return([5], [14], [17]);
671+
F1_B0:
672+
branch_align() -> ();
673+
drop<NonZero<felt252>>([13]) -> ();
674+
redeposit_gas([6]) -> ([18]);
675+
struct_deconstruct<test::Inner>([2]) -> ([19]);
676+
const_as_immediate<Const<felt252, 2>>() -> ([20]);
677+
felt252_mul([19], [20]) -> ([21]);
678+
const_as_immediate<Const<felt252, 1>>() -> ([22]);
679+
felt252_add([3], [22]) -> ([23]);
680+
struct_construct<test::Inner>([21]) -> ([24]);
681+
store_temp<RangeCheck>([5]) -> ([5]);
682+
store_temp<GasBuiltin>([18]) -> ([18]);
683+
store_temp<test::Inner>([24]) -> ([24]);
684+
store_temp<felt252>([23]) -> ([23]);
685+
store_temp<felt252>([4]) -> ([4]);
686+
function_call<user@test::loop_snap_mut[116-254]>([5], [18], [24], [23], [4]) -> ([25], [26], [27]);
687+
return([25], [26], [27]);
688+
F1_B1:
689+
branch_align() -> ();
690+
drop<felt252>([4]) -> ();
691+
drop<felt252>([3]) -> ();
692+
drop<test::Inner>([2]) -> ();
693+
function_call<user@core::panic_with_const_felt252::<375233589013918064796019>>() -> ([28]);
694+
enum_init<core::panics::PanicResult::<(test::Inner, core::felt252, ())>, 1>([28]) -> ([29]);
695+
store_temp<RangeCheck>([7]) -> ([7]);
696+
store_temp<GasBuiltin>([8]) -> ([8]);
697+
store_temp<core::panics::PanicResult::<(test::Inner, core::felt252, ())>>([29]) -> ([29]);
698+
return([7], [8], [29]);
699+
F2:
700+
const_as_immediate<Const<felt252, 375233589013918064796019>>() -> ([0]);
701+
store_temp<felt252>([0]) -> ([0]);
702+
function_call<user@core::panic_with_felt252>([0]) -> ([1]);
703+
return([1]);
704+
F3:
705+
array_new<felt252>() -> ([1]);
706+
array_append<felt252>([1], [0]) -> ([2]);
707+
struct_construct<core::panics::Panic>() -> ([3]);
708+
struct_construct<Tuple<core::panics::Panic, Array<felt252>>>([3], [2]) -> ([4]);
709+
store_temp<Tuple<core::panics::Panic, Array<felt252>>>([4]) -> ([4]);
710+
return([4]);
711+
712+
test::loop_snap_mut@F0([0]: RangeCheck, [1]: GasBuiltin, [2]: felt252) -> (RangeCheck, GasBuiltin, core::panics::PanicResult::<(core::felt252,)>);
713+
test::loop_snap_mut[116-254]@F1([0]: RangeCheck, [1]: GasBuiltin, [2]: test::Inner, [3]: felt252, [4]: felt252) -> (RangeCheck, GasBuiltin, core::panics::PanicResult::<(test::Inner, core::felt252, ())>);
714+
core::panic_with_const_felt252::<375233589013918064796019>@F2() -> (Tuple<core::panics::Panic, Array<felt252>>);
715+
core::panic_with_felt252@F3([0]: felt252) -> (Tuple<core::panics::Panic, Array<felt252>>);

0 commit comments

Comments
 (0)