Skip to content

feat(brillig): Hoist shared constants across functions to the global space#7216

Merged
vezenovm merged 27 commits intomasterfrom
mv/hoist-shared-consts-globally
Feb 18, 2025
Merged

feat(brillig): Hoist shared constants across functions to the global space#7216
vezenovm merged 27 commits intomasterfrom
mv/hoist-shared-consts-globally

Conversation

@vezenovm
Copy link
Contributor

@vezenovm vezenovm commented Jan 28, 2025 •
edited
Loading

Description

Problem*

Resolves #7109

Summary*

Before compiling Brillig globals we run an analysis to see which constants are shared across multiple functions. Those values are allocated along with the user-defined globals and then special cased when compiling regular functions. Any repeat constants across functions should be a part of the global read-only memory space and will live throughout the program.

  1. When building our BrilligGlobals context structure we now maintain a new map:
/// Maps entry point to constants 
hoisted_global_constants: HashMap<FunctionId, ConstantCounterMap>,
/// Mapping of a constant value and the number of functions in which it occurs
pub(crate) type ConstantCounterMap = HashMap<(FieldElement, NumericType), usize>;
  1. NB: I re-computed ConstantAllocation to determine which constant appears in both functions. This was technically recomputed as we calculate a ConstantAllocation when creating a FunctionContext. For now, I decided to recompute it as it has a minor affect on compilation, but just noting this is a place of optimization in the future.
  2. When compiling globals, we fetch the constants which appear in more than one function.
  3. After compiling the user specified globals, we compile the hoisted global constants.
  4. Maintain a separate map for entry points to their hoisted globals:
/// Final map that associates an entry point with any local function constants
/// that are shared and were hoisted to the global space.
/// This map is kept separate from `entry_point_globals_map` to clearly distinguish
/// the two types of globals.
entry_point_hoisted_globals_map: HashMap<FunctionId, HoistedConstantsToBrilligGlobals>,
pub(crate) type HoistedConstantsToBrilligGlobals =
    HashMap<(FieldElement, NumericType), BrilligVariable>;
  1. When compiling regular Brillig blocks we gate liveness analysis on checking whether a variable is actually a hoisted constant, not only on dfg.is_global.
  2. When converting a variable we escape early if it is a hoisted constant.

Additional Context

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@vezenovm vezenovm marked this pull request as draft January 28, 2025 22:57
@github-actions
Copy link
Contributor

github-actions bot commented Jan 28, 2025 •
edited
Loading

Changes to Brillig bytecode sizes

Generated at commit: 533bd7c2a3933654446d04303385c61b7f84501d, compared to commit: 119bf620005b52362e3aca9321b69e96e8a42fc0

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
trait_impl_base_type_inliner_min -10 ✅ -4.00%
brillig_calls_array_inliner_min -9 ✅ -4.09%
regression_unsafe_no_predicates_inliner_min -4 ✅ -4.35%
uhashmap_inliner_zero -328 ✅ -4.36%
brillig_calls_conditionals_inliner_min -6 ✅ -4.48%
brillig_calls_conditionals_inliner_zero -6 ✅ -4.48%
references_inliner_min -20 ✅ -4.75%
slices_inliner_min -122 ✅ -4.81%
simple_print_inliner_min -12 ✅ -5.17%
regression_3051_inliner_min -10 ✅ -5.18%
higher_order_functions_inliner_min -75 ✅ -5.54%
brillig_calls_inliner_min -11 ✅ -5.70%
hashmap_inliner_min -616 ✅ -6.27%
uhashmap_inliner_min -532 ✅ -6.36%
reference_counts_inliner_zero -79 ✅ -6.92%
debug_logs_inliner_min -763 ✅ -7.82%
prelude_inliner_min -19 ✅ -8.37%
reference_counts_inliner_min -128 ✅ -9.98%
Full diff report 👇
Program Brillig opcodes (+/-) %
hashmap_inliner_max 19,752 (-14) -0.07%
u128_inliner_max 2,645 (-3) -0.11%
nested_array_dynamic_inliner_min 1,952 (-3) -0.15%
nested_array_dynamic_inliner_zero 1,952 (-3) -0.15%
keccak256_inliner_zero 1,916 (-3) -0.16%
sha2_byte_inliner_zero 2,175 (-4) -0.18%
keccak256_inliner_min 1,614 (-3) -0.19%
regression_inliner_zero 943 (-2) -0.21%
ram_blowup_regression_inliner_zero 919 (-2) -0.22%
modulus_inliner_min 1,761 (-5) -0.28%
nested_array_in_slice_inliner_min 1,015 (-3) -0.29%
nested_array_in_slice_inliner_zero 1,015 (-3) -0.29%
array_dynamic_blackbox_input_inliner_zero 1,005 (-3) -0.30%
poseidon_bn254_hash_inliner_zero 4,671 (-15) -0.32%
poseidon_bn254_hash_width_3_inliner_zero 4,671 (-15) -0.32%
ecdsa_secp256k1_inliner_zero 898 (-3) -0.33%
brillig_cow_regression_inliner_zero 2,068 (-7) -0.34%
array_dynamic_nested_blackbox_input_inliner_zero 861 (-3) -0.35%
wildcard_type_inliner_min 284 (-1) -0.35%
poseidonsponge_x5_254_inliner_zero 3,015 (-11) -0.36%
array_dedup_regression_inliner_min 267 (-1) -0.37%
regression_5252_inliner_zero 3,387 (-14) -0.41%
brillig_cow_inliner_zero 238 (-1) -0.42%
sha256_brillig_performance_regression_inliner_zero 1,649 (-7) -0.42%
conditional_1_inliner_zero 1,101 (-5) -0.45%
regression_4449_inliner_zero 753 (-4) -0.53%
to_le_bytes_inliner_min 170 (-1) -0.58%
6_inliner_zero 818 (-5) -0.61%
array_dynamic_inliner_min 326 (-2) -0.61%
uhashmap_inliner_max 12,650 (-78) -0.61%
slice_coercion_inliner_min 486 (-3) -0.61%
assert_inliner_min 160 (-1) -0.62%
conditional_regression_661_inliner_zero 160 (-1) -0.62%
generics_inliner_zero 160 (-1) -0.62%
sha256_var_witness_const_regression_inliner_zero 789 (-5) -0.63%
poseidonsponge_x5_254_inliner_min 3,154 (-20) -0.63%
7_inliner_min 153 (-1) -0.65%
blake3_inliner_min 153 (-1) -0.65%
brillig_blake2s_inliner_min 150 (-1) -0.66%
array_sort_inliner_max 290 (-2) -0.68%
array_sort_inliner_zero 290 (-2) -0.68%
aes128_encrypt_inliner_min 434 (-3) -0.69%
bench_2_to_17_inliner_zero 288 (-2) -0.69%
poseidon_bn254_hash_width_3_inliner_min 5,018 (-38) -0.75%
poseidon_bn254_hash_inliner_min 5,018 (-38) -0.75%
sha256_regression_inliner_zero 4,798 (-37) -0.77%
aes128_encrypt_inliner_zero 388 (-3) -0.77%
conditional_regression_short_circuit_inliner_zero 898 (-7) -0.77%
side_effects_constrain_array_inliner_min 126 (-1) -0.79%
side_effects_constrain_array_inliner_zero 126 (-1) -0.79%
array_to_slice_inliner_zero 744 (-6) -0.80%
slice_regex_inliner_zero 1,672 (-14) -0.83%
embedded_curve_ops_inliner_zero 358 (-3) -0.83%
binary_operator_overloading_inliner_min 355 (-3) -0.84%
to_bytes_integration_inliner_min 229 (-2) -0.87%
reference_only_used_as_alias_inliner_min 335 (-3) -0.89%
sha256_var_size_regression_inliner_zero 1,075 (-10) -0.92%
brillig_cow_inliner_min 318 (-3) -0.93%
slice_loop_inliner_zero 317 (-3) -0.94%
sha256_brillig_performance_regression_inliner_min 1,791 (-17) -0.94%
array_eq_inliner_min 105 (-1) -0.94%
fold_distinct_return_inliner_min 101 (-1) -0.98%
poseidon2_inliner_zero 295 (-3) -1.01%
slices_inliner_zero 2,070 (-22) -1.05%
fold_complex_outputs_inliner_zero 551 (-6) -1.08%
struct_inputs_inliner_zero 251 (-3) -1.18%
regression_5252_inliner_min 3,551 (-43) -1.20%
conditional_regression_661_inliner_min 165 (-2) -1.20%
derive_inliner_zero 329 (-4) -1.20%
fold_2_to_17_inliner_zero 397 (-5) -1.24%
strings_inliner_zero 922 (-12) -1.28%
brillig_cow_regression_inliner_min 2,301 (-30) -1.29%
tuple_inputs_inliner_min 306 (-4) -1.29%
tuple_inputs_inliner_zero 306 (-4) -1.29%
binary_operator_overloading_inliner_zero 225 (-3) -1.32%
ecdsa_secp256k1_inliner_min 1,190 (-16) -1.33%
simple_2d_array_inliner_min 144 (-2) -1.37%
array_sort_inliner_min 350 (-5) -1.41%
brillig_arrays_inliner_min 140 (-2) -1.41%
sha2_byte_inliner_min 2,921 (-42) -1.42%
brillig_pedersen_inliner_min 549 (-8) -1.44%
pedersen_check_inliner_min 549 (-8) -1.44%
sha256_var_padding_regression_inliner_zero 2,874 (-42) -1.44%
conditional_1_inliner_min 1,386 (-21) -1.49%
ram_blowup_regression_inliner_min 1,097 (-17) -1.53%
sha256_var_size_regression_inliner_min 1,222 (-19) -1.53%
6_inliner_min 984 (-16) -1.60%
brillig_calls_array_inliner_zero 184 (-3) -1.60%
sha256_var_witness_const_regression_inliner_min 961 (-16) -1.64%
regression_6734_inliner_min 58 (-1) -1.69%
array_dynamic_nested_blackbox_input_inliner_min 1,159 (-20) -1.70%
struct_inputs_inliner_min 284 (-5) -1.73%
regression_inliner_min 1,079 (-19) -1.73%
to_be_bytes_inliner_min 226 (-4) -1.74%
array_dynamic_blackbox_input_inliner_min 1,239 (-22) -1.74%
debug_logs_inliner_zero 5,327 (-96) -1.77%
regression_4449_inliner_min 1,040 (-19) -1.79%
slice_loop_inliner_min 327 (-6) -1.80%
conditional_regression_short_circuit_inliner_min 1,081 (-20) -1.82%
loop_inliner_min 108 (-2) -1.82%
regression_4124_inliner_min 54 (-1) -1.82%
conditional_2_inliner_min 161 (-3) -1.83%
sha256_regression_inliner_min 5,175 (-98) -1.86%
higher_order_functions_inliner_zero 715 (-14) -1.92%
generics_inliner_min 239 (-5) -2.05%
brillig_rc_regression_6123_inliner_min 237 (-5) -2.07%
fold_numeric_generic_poseidon_inliner_zero 562 (-12) -2.09%
no_predicates_numeric_generic_poseidon_inliner_zero 562 (-12) -2.09%
7_function_inliner_zero 509 (-11) -2.12%
hint_black_box_inliner_zero 364 (-8) -2.15%
multi_scalar_mul_inliner_min 270 (-6) -2.17%
brillig_conditional_inliner_min 45 (-1) -2.17%
conditional_regression_547_inliner_min 44 (-1) -2.22%
nested_arrays_from_brillig_inliner_min 175 (-4) -2.23%
hash_to_field_inliner_min 213 (-5) -2.29%
regression_6674_1_inliner_min 292 (-7) -2.34%
hint_black_box_inliner_min 374 (-9) -2.35%
sha256_inliner_zero 1,203 (-29) -2.35%
sha256_inliner_min 1,352 (-33) -2.38%
global_var_regression_simple_inliner_min 163 (-4) -2.40%
slice_regex_inliner_min 2,072 (-52) -2.45%
7_function_inliner_min 634 (-16) -2.46%
brillig_nested_arrays_inliner_zero 227 (-6) -2.58%
brillig_nested_arrays_inliner_min 260 (-7) -2.62%
global_consts_inliner_min 258 (-7) -2.64%
simple_shield_inliner_min 840 (-23) -2.67%
u128_inliner_zero 1,898 (-52) -2.67%
regression_11294_inliner_min 471 (-13) -2.69%
loop_invariant_regression_inliner_min 179 (-5) -2.72%
sha256_var_padding_regression_inliner_min 3,104 (-88) -2.76%
regression_bignum_inliner_min 351 (-10) -2.77%
references_inliner_zero 240 (-7) -2.83%
regression_6674_3_inliner_min 842 (-25) -2.88%
fold_numeric_generic_poseidon_inliner_min 622 (-19) -2.96%
no_predicates_numeric_generic_poseidon_inliner_min 622 (-19) -2.96%
trait_impl_base_type_inliner_zero 64 (-2) -3.03%
regression_5045_inliner_min 217 (-7) -3.13%
fold_complex_outputs_inliner_min 664 (-22) -3.21%
regression_6674_2_inliner_min 299 (-10) -3.24%
inline_decompose_hint_brillig_call_inliner_min 235 (-8) -3.29%
fmtstr_with_global_inliner_min 117 (-4) -3.31%
fold_2_to_17_inliner_min 388 (-14) -3.48%
bench_2_to_17_inliner_min 357 (-13) -3.51%
slice_dynamic_index_inliner_min 2,688 (-98) -3.52%
brillig_acir_as_brillig_inliner_min 109 (-4) -3.54%
derive_inliner_min 572 (-21) -3.54%
u128_inliner_min 2,175 (-80) -3.55%
array_to_slice_inliner_min 981 (-37) -3.63%
embedded_curve_ops_inliner_min 496 (-19) -3.69%
poseidon2_inliner_min 364 (-14) -3.70%
hashmap_inliner_zero 7,534 (-309) -3.94%
strings_inliner_min 1,090 (-45) -3.96%
trait_impl_base_type_inliner_min 240 (-10) -4.00%
brillig_calls_array_inliner_min 211 (-9) -4.09%
regression_unsafe_no_predicates_inliner_min 88 (-4) -4.35%
uhashmap_inliner_zero 7,187 (-328) -4.36%
brillig_calls_conditionals_inliner_min 128 (-6) -4.48%
brillig_calls_conditionals_inliner_zero 128 (-6) -4.48%
references_inliner_min 401 (-20) -4.75%
slices_inliner_min 2,412 (-122) -4.81%
simple_print_inliner_min 220 (-12) -5.17%
regression_3051_inliner_min 183 (-10) -5.18%
higher_order_functions_inliner_min 1,280 (-75) -5.54%
brillig_calls_inliner_min 182 (-11) -5.70%
hashmap_inliner_min 9,214 (-616) -6.27%
uhashmap_inliner_min 7,828 (-532) -6.36%
reference_counts_inliner_zero 1,063 (-79) -6.92%
debug_logs_inliner_min 8,988 (-763) -7.82%
prelude_inliner_min 208 (-19) -8.37%
reference_counts_inliner_min 1,154 (-128) -9.98%

@github-actions
Copy link
Contributor

github-actions bot commented Jan 28, 2025 •
edited
Loading

Changes to number of Brillig opcodes executed

Generated at commit: 533bd7c2a3933654446d04303385c61b7f84501d, compared to commit: 119bf620005b52362e3aca9321b69e96e8a42fc0

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
regression_5045_inliner_min +3 ❌ +13.64%
regression_unsafe_no_predicates_inliner_min +2 ❌ +8.33%
references_inliner_min -27 ✅ -4.21%
higher_order_functions_inliner_min -101 ✅ -4.41%
brillig_calls_array_inliner_min -12 ✅ -4.46%
strings_inliner_min -138 ✅ -4.77%
regression_3051_inliner_min -10 ✅ -4.98%
simple_print_inliner_min -12 ✅ -5.00%
inline_decompose_hint_brillig_call_inliner_min -14 ✅ -5.11%
embedded_curve_ops_inliner_min -37 ✅ -5.56%
brillig_calls_inliner_min -11 ✅ -5.82%
brillig_calls_conditionals_inliner_min -14 ✅ -7.07%
brillig_calls_conditionals_inliner_zero -14 ✅ -7.07%
debug_logs_inliner_min -814 ✅ -7.89%
multi_scalar_mul_inliner_min -1,530 ✅ -7.99%
prelude_inliner_min -19 ✅ -8.09%
reference_counts_inliner_zero -138 ✅ -8.22%
reference_counts_inliner_min -168 ✅ -9.22%
Full diff report 👇
Program Brillig opcodes (+/-) %
regression_5045_inliner_min 25 (+3) +13.64%
regression_unsafe_no_predicates_inliner_min 26 (+2) +8.33%
modulus_inliner_min 18,016 (-5) -0.03%
keccak256_inliner_min 31,616 (-9) -0.03%
keccak256_inliner_zero 31,235 (-9) -0.03%
to_bytes_integration_inliner_min 4,249 (-2) -0.05%
poseidonsponge_x5_254_inliner_zero 207,255 (-121) -0.06%
regression_5252_inliner_zero 1,033,260 (-647) -0.06%
array_eq_inliner_min 1,202 (-1) -0.08%
7_inliner_min 1,061 (-1) -0.09%
blake3_inliner_min 1,061 (-1) -0.09%
brillig_blake2s_inliner_min 1,058 (-1) -0.09%
aes128_encrypt_inliner_zero 3,075 (-3) -0.10%
poseidon_bn254_hash_inliner_zero 183,719 (-229) -0.12%
poseidon_bn254_hash_width_3_inliner_zero 183,719 (-229) -0.12%
regression_inliner_zero 2,642 (-4) -0.15%
aes128_encrypt_inliner_min 3,479 (-6) -0.17%
poseidonsponge_x5_254_inliner_min 211,330 (-369) -0.17%
hashmap_inliner_max 48,293 (-88) -0.18%
to_be_bytes_inliner_min 2,126 (-4) -0.19%
regression_5252_inliner_min 1,056,152 (-1,997) -0.19%
poseidon_bn254_hash_inliner_min 189,234 (-368) -0.19%
poseidon_bn254_hash_width_3_inliner_min 189,234 (-368) -0.19%
wildcard_type_inliner_min 435 (-1) -0.23%
loop_invariant_regression_inliner_min 1,757 (-5) -0.28%
brillig_cow_inliner_zero 800 (-3) -0.37%
array_dynamic_inliner_min 505 (-2) -0.39%
array_dedup_regression_inliner_min 1,230 (-5) -0.40%
global_consts_inliner_min 1,702 (-7) -0.41%
array_dynamic_nested_blackbox_input_inliner_zero 4,438 (-22) -0.49%
hash_to_field_inliner_min 936 (-5) -0.53%
bench_2_to_17_inliner_zero 609,451 (-3,268) -0.53%
conditional_1_inliner_zero 5,480 (-31) -0.56%
sha256_var_witness_const_regression_inliner_zero 6,882 (-39) -0.56%
brillig_cow_inliner_min 1,407 (-8) -0.57%
reference_only_used_as_alias_inliner_min 495 (-3) -0.60%
slice_coercion_inliner_min 469 (-3) -0.64%
conditional_regression_661_inliner_zero 151 (-1) -0.66%
regression_inliner_min 2,968 (-21) -0.70%
side_effects_constrain_array_inliner_min 138 (-1) -0.72%
side_effects_constrain_array_inliner_zero 138 (-1) -0.72%
array_sort_inliner_max 515 (-4) -0.77%
array_sort_inliner_zero 515 (-4) -0.77%
assert_inliner_min 127 (-1) -0.78%
conditional_1_inliner_min 5,932 (-48) -0.80%
array_dynamic_nested_blackbox_input_inliner_min 4,874 (-40) -0.81%
poseidon2_inliner_zero 723 (-6) -0.82%
6_inliner_zero 4,402 (-37) -0.83%
fold_distinct_return_inliner_min 118 (-1) -0.84%
slices_inliner_zero 3,571 (-31) -0.86%
conditional_regression_short_circuit_inliner_zero 4,480 (-39) -0.86%
sha256_var_witness_const_regression_inliner_min 7,646 (-68) -0.88%
slice_loop_inliner_zero 1,261 (-12) -0.94%
ram_blowup_regression_inliner_zero 847,852 (-8,205) -0.96%
struct_inputs_inliner_zero 595 (-6) -1.00%
brillig_cow_regression_inliner_min 565,474 (-5,711) -1.00%
brillig_cow_regression_inliner_zero 561,902 (-5,686) -1.00%
ram_blowup_regression_inliner_min 851,585 (-8,730) -1.01%
sha2_byte_inliner_min 89,155 (-927) -1.03%
6_inliner_min 4,802 (-50) -1.03%
sha2_byte_inliner_zero 72,219 (-761) -1.04%
simple_shield_inliner_min 2,554 (-27) -1.05%
array_to_slice_inliner_zero 1,944 (-21) -1.07%
conditional_regression_short_circuit_inliner_min 4,905 (-53) -1.07%
sha256_brillig_performance_regression_inliner_zero 26,959 (-301) -1.10%
7_function_inliner_zero 2,414 (-27) -1.11%
sha256_brillig_performance_regression_inliner_min 27,333 (-308) -1.11%
sha256_inliner_zero 11,183 (-127) -1.12%
sha256_inliner_min 12,108 (-138) -1.13%
u128_inliner_max 23,627 (-276) -1.15%
brillig_arrays_inliner_min 171 (-2) -1.16%
loop_inliner_min 170 (-2) -1.16%
slice_loop_inliner_min 1,275 (-15) -1.16%
simple_2d_array_inliner_min 169 (-2) -1.17%
array_sort_inliner_min 670 (-8) -1.18%
conditional_regression_661_inliner_min 166 (-2) -1.19%
struct_inputs_inliner_min 651 (-8) -1.21%
binary_operator_overloading_inliner_min 397 (-5) -1.24%
regression_4449_inliner_zero 224,177 (-2,870) -1.26%
conditional_2_inliner_min 156 (-2) -1.27%
uhashmap_inliner_max 135,778 (-1,742) -1.27%
fold_complex_outputs_inliner_zero 932 (-12) -1.27%
brillig_pedersen_inliner_min 920 (-12) -1.29%
pedersen_check_inliner_min 920 (-12) -1.29%
7_function_inliner_min 2,590 (-34) -1.30%
generics_inliner_zero 222 (-3) -1.33%
nested_array_dynamic_inliner_min 3,562 (-51) -1.41%
nested_array_dynamic_inliner_zero 3,562 (-51) -1.41%
regression_6674_1_inliner_min 1,030 (-15) -1.44%
embedded_curve_ops_inliner_zero 402 (-6) -1.47%
ecdsa_secp256k1_inliner_zero 7,663 (-117) -1.50%
regression_4449_inliner_min 264,075 (-4,198) -1.56%
ecdsa_secp256k1_inliner_min 8,227 (-132) -1.58%
regression_11294_inliner_min 1,802 (-29) -1.58%
regression_6734_inliner_min 62 (-1) -1.59%
array_dynamic_blackbox_input_inliner_zero 21,501 (-348) -1.59%
array_dynamic_blackbox_input_inliner_min 22,547 (-389) -1.70%
nested_array_in_slice_inliner_min 1,712 (-30) -1.72%
nested_array_in_slice_inliner_zero 1,712 (-30) -1.72%
debug_logs_inliner_zero 5,351 (-96) -1.76%
slice_dynamic_index_inliner_min 4,887 (-89) -1.79%
hint_black_box_inliner_zero 988 (-18) -1.79%
regression_6674_2_inliner_min 1,034 (-19) -1.80%
derive_inliner_zero 325 (-6) -1.81%
regression_bignum_inliner_min 541 (-10) -1.81%
regression_4124_inliner_min 54 (-1) -1.82%
fold_numeric_generic_poseidon_inliner_zero 5,613 (-106) -1.85%
no_predicates_numeric_generic_poseidon_inliner_zero 5,613 (-106) -1.85%
tuple_inputs_inliner_min 635 (-12) -1.85%
tuple_inputs_inliner_zero 635 (-12) -1.85%
nested_arrays_from_brillig_inliner_min 211 (-4) -1.86%
hint_black_box_inliner_min 1,002 (-19) -1.86%
global_var_regression_simple_inliner_min 210 (-4) -1.87%
brillig_rc_regression_6123_inliner_min 367 (-7) -1.87%
fold_numeric_generic_poseidon_inliner_min 5,747 (-111) -1.89%
no_predicates_numeric_generic_poseidon_inliner_min 5,747 (-111) -1.89%
slice_regex_inliner_zero 3,963 (-78) -1.93%
regression_6674_3_inliner_min 1,965 (-40) -2.00%
bench_2_to_17_inliner_min 697,706 (-14,709) -2.06%
fold_2_to_17_inliner_min 1,327,077 (-27,980) -2.06%
brillig_nested_arrays_inliner_zero 281 (-6) -2.09%
higher_order_functions_inliner_zero 1,300 (-28) -2.11%
fold_2_to_17_inliner_zero 1,289,660 (-27,962) -2.12%
generics_inliner_min 321 (-7) -2.13%
binary_operator_overloading_inliner_zero 225 (-5) -2.17%
uhashmap_inliner_zero 171,423 (-3,955) -2.26%
array_to_slice_inliner_min 2,209 (-52) -2.30%
brillig_conditional_inliner_min 41 (-1) -2.38%
conditional_regression_547_inliner_min 41 (-1) -2.38%
slice_regex_inliner_min 8,426 (-208) -2.41%
sha256_var_size_regression_inliner_zero 21,781 (-538) -2.41%
sha256_var_padding_regression_inliner_zero 236,049 (-5,834) -2.41%
sha256_var_size_regression_inliner_min 22,512 (-562) -2.44%
sha256_var_padding_regression_inliner_min 237,412 (-5,970) -2.45%
hashmap_inliner_zero 72,781 (-1,850) -2.48%
fold_complex_outputs_inliner_min 1,039 (-28) -2.62%
poseidon2_inliner_min 850 (-23) -2.63%
u128_inliner_min 46,991 (-1,274) -2.64%
brillig_calls_array_inliner_zero 218 (-6) -2.68%
sha256_regression_inliner_zero 159,382 (-4,422) -2.70%
sha256_regression_inliner_min 161,332 (-4,514) -2.72%
trait_impl_base_type_inliner_zero 68 (-2) -2.86%
brillig_nested_arrays_inliner_min 454 (-14) -2.99%
strings_inliner_zero 2,035 (-63) -3.00%
uhashmap_inliner_min 191,226 (-5,935) -3.01%
slices_inliner_min 4,159 (-130) -3.03%
u128_inliner_zero 39,481 (-1,244) -3.05%
derive_inliner_min 596 (-19) -3.09%
fmtstr_with_global_inliner_min 120 (-4) -3.23%
brillig_acir_as_brillig_inliner_min 113 (-4) -3.42%
hashmap_inliner_min 87,278 (-3,144) -3.48%
references_inliner_zero 380 (-14) -3.55%
trait_impl_base_type_inliner_min 253 (-10) -3.80%
references_inliner_min 615 (-27) -4.21%
higher_order_functions_inliner_min 2,190 (-101) -4.41%
brillig_calls_array_inliner_min 257 (-12) -4.46%
strings_inliner_min 2,755 (-138) -4.77%
regression_3051_inliner_min 191 (-10) -4.98%
simple_print_inliner_min 228 (-12) -5.00%
inline_decompose_hint_brillig_call_inliner_min 260 (-14) -5.11%
embedded_curve_ops_inliner_min 629 (-37) -5.56%
brillig_calls_inliner_min 178 (-11) -5.82%
brillig_calls_conditionals_inliner_min 184 (-14) -7.07%
brillig_calls_conditionals_inliner_zero 184 (-14) -7.07%
debug_logs_inliner_min 9,503 (-814) -7.89%
multi_scalar_mul_inliner_min 17,608 (-1,530) -7.99%
prelude_inliner_min 216 (-19) -8.09%
reference_counts_inliner_zero 1,541 (-138) -8.22%
reference_counts_inliner_min 1,654 (-168) -9.22%

github-actions[bot]
github-actions bot reviewed Jan 29, 2025
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 871811d Previous: a9e9850 Ratio
noir-lang_schnorr_ 1 s 0 s +∞
noir-lang_noir_sort_ 1 s 0 s +∞
AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob 64 s 51 s 1.25

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

vezenovm
vezenovm commented Jan 30, 2025
View reviewed changes
github-actions[bot]
github-actions bot reviewed Jan 30, 2025
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Compilation Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 40c3b5b Previous: 49d1b13 Ratio
sha256_regression 1.23 s 0.977 s 1.26
regression_4709 1.04 s 0.845 s 1.23
global_var_regression_entry_points 0.74 s 0.566 s 1.31

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

vezenovm
vezenovm commented Jan 30, 2025
View reviewed changes
vezenovm
vezenovm commented Jan 30, 2025
View reviewed changes
@vezenovm vezenovm marked this pull request as ready for review January 30, 2025 13:06
@vezenovm vezenovm requested a review from a team January 30, 2025 13:12
vezenovm
vezenovm commented Jan 30, 2025
View reviewed changes
vezenovm
vezenovm commented Jan 30, 2025
View reviewed changes
michaeljklein
michaeljklein suggested changes Jan 30, 2025
View reviewed changes
@TomAFrench
Copy link
Member

TomAFrench commented Jan 30, 2025

Can you explain a bit more on why we don't place these hoisted constants into the main globals map? We should be able to treat them in exactly the same way.

@vezenovm
Copy link
Contributor Author

vezenovm commented Jan 30, 2025 •
edited
Loading

Can you explain a bit more on why we don't place these hoisted constants into the main globals map? We should be able to treat them in exactly the same way.

I decided to do it during Brillig for a few reasons aside this being a Brillig unique operation.

  1. To avoid having to alter the globals graph for every function and the cloning cost that comes with it. We would also have to add the ability to mutate the normal function DFG. This feels like a leakage for a Brillig exclusive feature.
  2. I also wanted to re-use some of the pre-existing Brillig constant analysis tools.
  3. Doing this during our existing Brillig globals generation we could avoid another SSA pass or making a pre-existing complex more complex (as each pass is currently per-function).

Due to the above reasons, I felt we could do this as part of Brillig gen rather than having to alter the pre-existing SSA.

I had considered placing them in the main globals map as well, but ultimately decided to trade-off a new "type" of global and a little extra complexity in Brillig to avoid extra complexity in the SSA passes (most likely it would be constant folding that would be updated) for this Brillig unique feature.

michaeljklein
michaeljklein approved these changes Feb 5, 2025
View reviewed changes
Copy link
Contributor

@michaeljklein michaeljklein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

TomAFrench
TomAFrench requested changes Feb 6, 2025
View reviewed changes
Copy link
Member

@TomAFrench TomAFrench left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocking this waiting for #7277

@TomAFrench
Copy link
Member

TomAFrench commented Feb 18, 2025

Just needs conflicts to be addressed now.

TomAFrench
TomAFrench reviewed Feb 18, 2025
View reviewed changes
TomAFrench
TomAFrench reviewed Feb 18, 2025
View reviewed changes
vezenovm and others added 2 commits February 18, 2025 19:16
@vezenovm
nit cleanup
c739199
@vezenovm @TomAFrench
Update compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
8c8bf04 
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
TomAFrench
TomAFrench reviewed Feb 18, 2025
View reviewed changes
@vezenovm @TomAFrench
Update compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
d54d418 
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
@vezenovm vezenovm enabled auto-merge February 18, 2025 19:35
@vezenovm vezenovm added this pull request to the merge queue Feb 18, 2025
Merged via the queue into master with commit 8652072 Feb 18, 2025
102 checks passed
@vezenovm vezenovm deleted the mv/hoist-shared-consts-globally branch February 18, 2025 19:57
@noirwhal noirwhal mentioned this pull request Feb 18, 2025
Merged
TomAFrench added a commit that referenced this pull request Feb 19, 2025
@TomAFrench
Merge branch 'master' into tf/simplify-reports
48de313 
* master:
  fix(performance): Remove redundant slice access check from brillig (#7434)
  chore(docs): updating tutorials and other nits to beta.2 (#7405)
  feat: LSP hover for integer literals (#7368)
  feat(experimental): Compile match expressions (#7312)
  feat(acir_field): Add little-endian byte serialization for FieldElement (#7258)
  feat: allow unquoting TraitConstraint in trait impl position (#7395)
  feat(brillig): Hoist shared constants across functions to the global space (#7216)
  feat(LSP): auto-import via visible reexport (#7409)
  fix(brillig): Brillig entry point analysis and function specialization through duplication (#7277)
  chore: redo typo PR by maximevtush (#7425)
  fix(ssa): Accurately mark binary ops for hoisting and check Div/Mod against induction variable lower bound (#7396)
  feat!: remove bigint from stdlib (#7411)
AztecBot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Feb 19, 2025
[1 changes] fix(performance): Remove redundant slice access check fro…
c80639a 
…m brillig (noir-lang/noir#7434)

chore(docs): updating tutorials and other nits to beta.2 (noir-lang/noir#7405)
feat: LSP hover for integer literals (noir-lang/noir#7368)
feat(experimental): Compile match expressions (noir-lang/noir#7312)
feat(acir_field): Add little-endian byte serialization for FieldElement (noir-lang/noir#7258)
feat: allow unquoting TraitConstraint in trait impl position (noir-lang/noir#7395)
feat(brillig): Hoist shared constants across functions to the global space (noir-lang/noir#7216)
feat(LSP): auto-import via visible reexport (noir-lang/noir#7409)
fix(brillig): Brillig entry point analysis and function specialization through duplication (noir-lang/noir#7277)
chore: redo typo PR by maximevtush (noir-lang/noir#7425)
fix(ssa): Accurately mark binary ops for hoisting and check Div/Mod against induction variable lower bound (noir-lang/noir#7396)
feat!: remove bigint from stdlib (noir-lang/noir#7411)
chore: bump aztec-packages commit (noir-lang/noir#7415)
chore: deprecate `merkle` module of stdlib (noir-lang/noir#7413)
chore(ci): lock aztec-packages commit in CI (noir-lang/noir#7414)
feat: while statement (noir-lang/noir#7280)
AztecBot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Feb 19, 2025
fix(performance): Remove redundant slice access check from brillig (n…
378dd60 
…oir-lang/noir#7434)

chore(docs): updating tutorials and other nits to beta.2 (noir-lang/noir#7405)
feat: LSP hover for integer literals (noir-lang/noir#7368)
feat(experimental): Compile match expressions (noir-lang/noir#7312)
feat(acir_field): Add little-endian byte serialization for FieldElement (noir-lang/noir#7258)
feat: allow unquoting TraitConstraint in trait impl position (noir-lang/noir#7395)
feat(brillig): Hoist shared constants across functions to the global space (noir-lang/noir#7216)
feat(LSP): auto-import via visible reexport (noir-lang/noir#7409)
fix(brillig): Brillig entry point analysis and function specialization through duplication (noir-lang/noir#7277)
chore: redo typo PR by maximevtush (noir-lang/noir#7425)
fix(ssa): Accurately mark binary ops for hoisting and check Div/Mod against induction variable lower bound (noir-lang/noir#7396)
feat!: remove bigint from stdlib (noir-lang/noir#7411)
chore: bump aztec-packages commit (noir-lang/noir#7415)
chore: deprecate `merkle` module of stdlib (noir-lang/noir#7413)
chore(ci): lock aztec-packages commit in CI (noir-lang/noir#7414)
feat: while statement (noir-lang/noir#7280)
@AztecBot AztecBot mentioned this pull request Feb 19, 2025
Merged
TomAFrench added a commit to AztecProtocol/aztec-packages that referenced this pull request Feb 19, 2025
@AztecBot @TomAFrench
feat: Sync from noir (#12064)
fdd5375 
Automated pull of development from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix(performance): Remove redundant slice access check from brillig
(noir-lang/noir#7434)
chore(docs): updating tutorials and other nits to beta.2
(noir-lang/noir#7405)
feat: LSP hover for integer literals
(noir-lang/noir#7368)
feat(experimental): Compile match expressions
(noir-lang/noir#7312)
feat(acir_field): Add little-endian byte serialization for FieldElement
(noir-lang/noir#7258)
feat: allow unquoting TraitConstraint in trait impl position
(noir-lang/noir#7395)
feat(brillig): Hoist shared constants across functions to the global
space (noir-lang/noir#7216)
feat(LSP): auto-import via visible reexport
(noir-lang/noir#7409)
fix(brillig): Brillig entry point analysis and function specialization
through duplication (noir-lang/noir#7277)
chore: redo typo PR by maximevtush
(noir-lang/noir#7425)
fix(ssa): Accurately mark binary ops for hoisting and check Div/Mod
against induction variable lower bound
(noir-lang/noir#7396)
feat!: remove bigint from stdlib
(noir-lang/noir#7411)
chore: bump aztec-packages commit
(noir-lang/noir#7415)
chore: deprecate `merkle` module of stdlib
(noir-lang/noir#7413)
chore(ci): lock aztec-packages commit in CI
(noir-lang/noir#7414)
feat: while statement (noir-lang/noir#7280)
END_COMMIT_OVERRIDE

---------

Co-authored-by: Tom French <tom@tomfren.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@TomAFrench TomAFrench TomAFrench approved these changes

@sirasistant sirasistant Awaiting requested review from sirasistant

@jfecher jfecher Awaiting requested review from jfecher

+1 more reviewer

@michaeljklein michaeljklein michaeljklein approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Hoist constants shared across functions into the global space

3 participants

@vezenovm @TomAFrench @michaeljklein