fix: Reproduce and fix bytecode blowup #6972
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Here's a flame graph of the problematic contract in
To reproduce: PROGRAM_DIR=$PWD/noir-projects/noir-contracts
cd noir/noir-repo
git checkout origin/12-09-chore_removing_noir_bug_workaround
export GIT_DIRTY=true
export GIT_COMMIT=$(git rev-parse HEAD)
cargo run --release -p nargo_cli -- --program-dir $PROGRAM_DIR compile --package token_contract
ls -lh $PROGRAM_DIR/target/token_contract-Token.json
jq '.functions | sort_by(.name) | .[] | {name: .name, size: .bytecode | length }' $PROGRAM_DIR/target/token_contract-Token.json
$PROGRAM_DIR/scripts/flamegraph.sh Token transfer_to_publicThe SVG is at |
Execution Report
|
Compilation Report
|
|
Here's the baseline flame graph for reference:
|
Compilation Memory Report
|
Execution Memory Report
|
vezenovm
reviewed
Jan 7, 2025
test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr
Outdated
Show resolved
Hide resolved
|
I am looking at the original SSA now, printed by this command: cargo run -p nargo_cli -- --program-dir $PROGRAM_DIR compile --package token_contract --show-ssa-pass-name "Array Set Optimizations" > compile-output.txtIt's 2 million lines, but looking for the first occurrence of "ciphertext length mismatch" reveals a pattern like this: I suspect that explain all the RANGE checks, coming from check_unsigned_overflow after inserting the multiplication. |
|
|
|
@TomAFrench that's what I thought, on the other hand isn't it a I'm looking at the ACIR and see this pattern repeating: And then another array gets initialised with |
Ah, the |
|
Yep, we can handle this more cleanly after #6979 though. |
|
I added a simple print and see tons of this kind of range checks: Changed the code like this, adding the BinaryOp::Mul => {
if bit_size == 1
|| max_lhs_bits + max_rhs_bits <= bit_size
|| max_lhs_bits == 1
|| max_rhs_bits == 1
{
// Either performing boolean multiplication (which cannot overflow),
// or `lhs` and `rhs` have both been casted up from smaller types and so cannot overflow.
return Ok(());
}
"attempt to multiply with overflow".to_string()
}That brings the code size down from 50MB to 30MB, which is still higher than expected. Need to investigate the repeated arrays. But that change seem correct, doesn't it? |
|
@vezenovm I think you were right, it does seem to copy the array on each iteration, it is visible in the SSA. Is there a way to nudge it towards unrolling it with the use of array writes instead? Could the side effect variable be preventing it somehow? ...
// Make an array from the current variables
v216516 = make_array [v215463, v215465, v215467, v215469, v215471, v215473, v215475, v215477, v215479, v215481, v215483, v215485, v215487, v215489, v215491, v215493, v215495, v215497, v215499, v215501, v215503, v215505, v215507, v215509, v215511, v215513, v215515, v215517, v215519, v215521, v215523, v215525, v215527, v215529, v215531, v215533, v215535, v215537, v215539, v215541, v215543, v215545, v215547, v215549, v215551, v215553, v215555, v215557, v215559, v215561, v215563, v215565, v215567, v215569, v215571, v215573, v215575, v215577, v215579, v215581, v215583, v215585, v215587, v215589, v215591, v215593, v215595, v215597, v215599, v215601, v215603, v215605, v215607, v215609, v215611, v215613, v215615, v215617, v215619, v215621, v215623, v215625, v215627, v215629, v215631, v215633, v215635, v215637, v215639, v215641, v215643, v215645, v215647, v215649, v215651, v215653, v215655, v215657, v215659, v215661, v215663, v215665, v215667, v215669, v215671, v215673, v215675, v215677, v215679, v215681, v215683, v215685, v215687, v215689, v215691, v215693, v215695, v215697, v215699, v215701, v215703, v215705, v215707, v215709, v215711, v215713, v215715, v215717, v215719, v215721, v215723, v215725, v215727, v215729, v215731, v215733, v215735, v215737, v215739, v215741, v215743, v215745, v215747, v215749, v215751, v215753, v215755, v215757, v215759, v215761, v215763, v215765, v215767, v215769, v215771, v215773, v215775, v215777, v215779, v215781, v215783, v215785, v215787, v215789, v215791, v215793, v215795, v215797, v215799, v215801, v215803, v215805, v215807, v215809, v215811, v215813, v215815, v215817, v215819, v215821, v215823, v215825, v215827, v215829, v215831, v215833, v215835, v215837, v215839, v215841, v215843, v215845, v215847, v215849, v215851, v215853, v215855, v215857, v215859, v215861, v215863, v215865, v215867, v215869, v215871, v215873, v215875, v215877, v215879, v215881, v215883, v215885, v215887, v215889, v215891, v215893, v215895, v215897, v215899, v215901, v215903, v215905, v215907, v215909, v215911, v215913, v215915, v215917, v215919, v215921, v215923, v215925, v215927, v215929, v215931, v215933, v215935, v215937, v215939, v215941, v215943, v215945, v215947, v215949, v215951, v215953, v215955, v215957, v215959, v215961, v215963, v215965, v215967, v215969, v215971, v215973, v215975, v215977, v215979, v215981, v215983, v215985, v215987, v215989, v215991, v215993, v215995, v215997, v215999, v216001, v216003, v216005, v216007, v216009, v216011, v216013, v216015, v216017, v216019, v216021, v216023, v216025, v216027, v216029, v216031, v216033, v216035, v216037, v216039, v216041, v216043, v216045, v216047, v216049, v216051, v216053, v216055, v216057, v216059, v216061, v216063, v216065, v216067, v216069, v216071, v216073, v216075, v216077, v216079, v216081, v216083, v216085, v216087, v216089, v216091, v216093, v216095, v216097, v216099, v216101, v216103, v216105, v216107, v216109, v216111, v216113, v216115, v216117, v216119, v216121, v216123, v216125, v216127, v216129, v216131, v216133, v216135, v216137, v216139, v216141, v216143, v216145, v216147, v216149, v216151, v216153, v216155, v216157, v216159, v216161, v216163, v216165, v216167, v216169, v216171, v216173, v216175, v216177, v216179, v216181, v216183, v216185, v216187, v216189, v216191, v216193, v216195, v216197, v216199, v216201, v216203, v216205, v216207, v216209, v216211, v216213, v216215, v216217, v216219, v216221, v216223, v216225, v216227, v216229, v216231, v216233, v216235, v216237, v216239, v216241, v216243, v216245, v216247, v216249, v216251, v216253, v216255, v216257, v216259, v216261, v216263, v216265, v216267, v216269, v216271, v216273, v216275, v216277, v216279, v216281, v216283, v216285, v216287, v216289, v216291, v216293, v216295, v216297, v216299, v216301, v216303, v216305, v216307, v216309, v216311, v216313, v216315, v216317, v216319, v216321, v216323, v216325, v216327, v216329, v216331, v216333, v216335, v216337, v216339, v216341, v216343, v216345, v216347, v216349, v216351, v216353, v216355, v216357, v216359, v216361, v216363, v216365, v216367, v216369, v216371, v216373, v216375, v216377, v216379, v216381, v216383, v216385, v216387, v216389, v216391, v216393, v216395, v216397, v216399, v216401, v216403, v216405, v216407, v216409, v216411, v216413, v216415, v216417, v216419, v216421, v216423, v216425, v216427, v216429, v216431, v216433, v216435, v216437, v216439, v216441, v216443, v216445, v216447, v216449, v216451, v216453, v216455, v216457, v216459, v216461, v216463, v216465, v216467, v216469, v216471, v216473, v216475, v216477, v216479, v216481, v216483, v216485, v216487, v216489, v216491, v216493, v216495, v216497, v216499, v216501, v216503, v216505, v216507, v216509, v216511, v216513, v216515] : [u8; 527]
// Maybe this is [offset+i] where i=177
v216517 = add v28449, u32 177
v216518 = lt u32 177, v27866
v216519 = mul v216518, v22441
constrain v216519 == v22441, "Index out of bounds"
// read incoming_body_ciphertext[i]; ie. read item 177 for copying into the output
v216520 = array_get v27867, index u32 177 -> u8
// write encrypted_bytes[offset + i]
v216521 = array_set mut v216516, index v216517, value v216520
// Now read all values from the array and multiply with the side effect variable
v216522 = array_get v216521, index Field 0 -> u8
v216523 = mul v28367, v216522
...
v217573 = mul v28367, v217572
v217574 = array_get v216521, index Field 526 -> u8
v217575 = mul v28367, v217574
// Create a new array
v217576 = make_array [v216523, v216525, v216527, v216529, v216531, v216533, v216535, v216537, v216539, v216541, v216543, v216545, v216547, v216549, v216551, v216553, v216555, v216557, v216559, v216561, v216563, v216565, v216567, v216569, v216571, v216573, v216575, v216577, v216579, v216581, v216583, v216585, v216587, v216589, v216591, v216593, v216595, v216597, v216599, v216601, v216603, v216605, v216607, v216609, v216611, v216613, v216615, v216617, v216619, v216621, v216623, v216625, v216627, v216629, v216631, v216633, v216635, v216637, v216639, v216641, v216643, v216645, v216647, v216649, v216651, v216653, v216655, v216657, v216659, v216661, v216663, v216665, v216667, v216669, v216671, v216673, v216675, v216677, v216679, v216681, v216683, v216685, v216687, v216689, v216691, v216693, v216695, v216697, v216699, v216701, v216703, v216705, v216707, v216709, v216711, v216713, v216715, v216717, v216719, v216721, v216723, v216725, v216727, v216729, v216731, v216733, v216735, v216737, v216739, v216741, v216743, v216745, v216747, v216749, v216751, v216753, v216755, v216757, v216759, v216761, v216763, v216765, v216767, v216769, v216771, v216773, v216775, v216777, v216779, v216781, v216783, v216785, v216787, v216789, v216791, v216793, v216795, v216797, v216799, v216801, v216803, v216805, v216807, v216809, v216811, v216813, v216815, v216817, v216819, v216821, v216823, v216825, v216827, v216829, v216831, v216833, v216835, v216837, v216839, v216841, v216843, v216845, v216847, v216849, v216851, v216853, v216855, v216857, v216859, v216861, v216863, v216865, v216867, v216869, v216871, v216873, v216875, v216877, v216879, v216881, v216883, v216885, v216887, v216889, v216891, v216893, v216895, v216897, v216899, v216901, v216903, v216905, v216907, v216909, v216911, v216913, v216915, v216917, v216919, v216921, v216923, v216925, v216927, v216929, v216931, v216933, v216935, v216937, v216939, v216941, v216943, v216945, v216947, v216949, v216951, v216953, v216955, v216957, v216959, v216961, v216963, v216965, v216967, v216969, v216971, v216973, v216975, v216977, v216979, v216981, v216983, v216985, v216987, v216989, v216991, v216993, v216995, v216997, v216999, v217001, v217003, v217005, v217007, v217009, v217011, v217013, v217015, v217017, v217019, v217021, v217023, v217025, v217027, v217029, v217031, v217033, v217035, v217037, v217039, v217041, v217043, v217045, v217047, v217049, v217051, v217053, v217055, v217057, v217059, v217061, v217063, v217065, v217067, v217069, v217071, v217073, v217075, v217077, v217079, v217081, v217083, v217085, v217087, v217089, v217091, v217093, v217095, v217097, v217099, v217101, v217103, v217105, v217107, v217109, v217111, v217113, v217115, v217117, v217119, v217121, v217123, v217125, v217127, v217129, v217131, v217133, v217135, v217137, v217139, v217141, v217143, v217145, v217147, v217149, v217151, v217153, v217155, v217157, v217159, v217161, v217163, v217165, v217167, v217169, v217171, v217173, v217175, v217177, v217179, v217181, v217183, v217185, v217187, v217189, v217191, v217193, v217195, v217197, v217199, v217201, v217203, v217205, v217207, v217209, v217211, v217213, v217215, v217217, v217219, v217221, v217223, v217225, v217227, v217229, v217231, v217233, v217235, v217237, v217239, v217241, v217243, v217245, v217247, v217249, v217251, v217253, v217255, v217257, v217259, v217261, v217263, v217265, v217267, v217269, v217271, v217273, v217275, v217277, v217279, v217281, v217283, v217285, v217287, v217289, v217291, v217293, v217295, v217297, v217299, v217301, v217303, v217305, v217307, v217309, v217311, v217313, v217315, v217317, v217319, v217321, v217323, v217325, v217327, v217329, v217331, v217333, v217335, v217337, v217339, v217341, v217343, v217345, v217347, v217349, v217351, v217353, v217355, v217357, v217359, v217361, v217363, v217365, v217367, v217369, v217371, v217373, v217375, v217377, v217379, v217381, v217383, v217385, v217387, v217389, v217391, v217393, v217395, v217397, v217399, v217401, v217403, v217405, v217407, v217409, v217411, v217413, v217415, v217417, v217419, v217421, v217423, v217425, v217427, v217429, v217431, v217433, v217435, v217437, v217439, v217441, v217443, v217445, v217447, v217449, v217451, v217453, v217455, v217457, v217459, v217461, v217463, v217465, v217467, v217469, v217471, v217473, v217475, v217477, v217479, v217481, v217483, v217485, v217487, v217489, v217491, v217493, v217495, v217497, v217499, v217501, v217503, v217505, v217507, v217509, v217511, v217513, v217515, v217517, v217519, v217521, v217523, v217525, v217527, v217529, v217531, v217533, v217535, v217537, v217539, v217541, v217543, v217545, v217547, v217549, v217551, v217553, v217555, v217557, v217559, v217561, v217563, v217565, v217567, v217569, v217571, v217573, v217575] : [u8; 527]
// Compute `offset + i`
v217577 = add v28449, u32 178
v217578 = lt u32 178, v27866
v217579 = mul v217578, v22441
constrain v217579 == v22441, "Index out of bounds"
// Read item 178 from incoming_body_ciphertext
v217580 = array_get v27867, index u32 178 -> u8
// Write to encrypted_bytes
v217581 = array_set mut v217576, index v217577, value v217580
// Read all items again and multiply with the same side effect variable
v217582 = array_get v217581, index Field 0 -> u8
v217583 = mul v28367, v217582
v217584 = array_get v217581, index Field 1 -> u8
v217585 = mul v28367, v217584
...
v218634 = array_get v217581, index Field 526 -> u8
v218635 = mul v28367, v218634
v218636 = make_array [v217583, v217585, v217587, v217589, v217591, v217593, v217595, v217597, v217599, v217601, v217603, v217605, v217607, v217609, v217611, v217613, v217615, v217617, v217619, v217621, v217623, v217625, v217627, v217629, v217631, v217633, v217635, v217637, v217639, v217641, v217643, v217645, v217647, v217649, v217651, v217653, v217655, v217657, v217659, v217661, v217663, v217665, v217667, v217669, v217671, v217673, v217675, v217677, v217679, v217681, v217683, v217685, v217687, v217689, v217691, v217693, v217695, v217697, v217699, v217701, v217703, v217705, v217707, v217709, v217711, v217713, v217715, v217717, v217719, v217721, v217723, v217725, v217727, v217729, v217731, v217733, v217735, v217737, v217739, v217741, v217743, v217745, v217747, v217749, v217751, v217753, v217755, v217757, v217759, v217761, v217763, v217765, v217767, v217769, v217771, v217773, v217775, v217777, v217779, v217781, v217783, v217785, v217787, v217789, v217791, v217793, v217795, v217797, v217799, v217801, v217803, v217805, v217807, v217809, v217811, v217813, v217815, v217817, v217819, v217821, v217823, v217825, v217827, v217829, v217831, v217833, v217835, v217837, v217839, v217841, v217843, v217845, v217847, v217849, v217851, v217853, v217855, v217857, v217859, v217861, v217863, v217865, v217867, v217869, v217871, v217873, v217875, v217877, v217879, v217881, v217883, v217885, v217887, v217889, v217891, v217893, v217895, v217897, v217899, v217901, v217903, v217905, v217907, v217909, v217911, v217913, v217915, v217917, v217919, v217921, v217923, v217925, v217927, v217929, v217931, v217933, v217935, v217937, v217939, v217941, v217943, v217945, v217947, v217949, v217951, v217953, v217955, v217957, v217959, v217961, v217963, v217965, v217967, v217969, v217971, v217973, v217975, v217977, v217979, v217981, v217983, v217985, v217987, v217989, v217991, v217993, v217995, v217997, v217999, v218001, v218003, v218005, v218007, v218009, v218011, v218013, v218015, v218017, v218019, v218021, v218023, v218025, v218027, v218029, v218031, v218033, v218035, v218037, v218039, v218041, v218043, v218045, v218047, v218049, v218051, v218053, v218055, v218057, v218059, v218061, v218063, v218065, v218067, v218069, v218071, v218073, v218075, v218077, v218079, v218081, v218083, v218085, v218087, v218089, v218091, v218093, v218095, v218097, v218099, v218101, v218103, v218105, v218107, v218109, v218111, v218113, v218115, v218117, v218119, v218121, v218123, v218125, v218127, v218129, v218131, v218133, v218135, v218137, v218139, v218141, v218143, v218145, v218147, v218149, v218151, v218153, v218155, v218157, v218159, v218161, v218163, v218165, v218167, v218169, v218171, v218173, v218175, v218177, v218179, v218181, v218183, v218185, v218187, v218189, v218191, v218193, v218195, v218197, v218199, v218201, v218203, v218205, v218207, v218209, v218211, v218213, v218215, v218217, v218219, v218221, v218223, v218225, v218227, v218229, v218231, v218233, v218235, v218237, v218239, v218241, v218243, v218245, v218247, v218249, v218251, v218253, v218255, v218257, v218259, v218261, v218263, v218265, v218267, v218269, v218271, v218273, v218275, v218277, v218279, v218281, v218283, v218285, v218287, v218289, v218291, v218293, v218295, v218297, v218299, v218301, v218303, v218305, v218307, v218309, v218311, v218313, v218315, v218317, v218319, v218321, v218323, v218325, v218327, v218329, v218331, v218333, v218335, v218337, v218339, v218341, v218343, v218345, v218347, v218349, v218351, v218353, v218355, v218357, v218359, v218361, v218363, v218365, v218367, v218369, v218371, v218373, v218375, v218377, v218379, v218381, v218383, v218385, v218387, v218389, v218391, v218393, v218395, v218397, v218399, v218401, v218403, v218405, v218407, v218409, v218411, v218413, v218415, v218417, v218419, v218421, v218423, v218425, v218427, v218429, v218431, v218433, v218435, v218437, v218439, v218441, v218443, v218445, v218447, v218449, v218451, v218453, v218455, v218457, v218459, v218461, v218463, v218465, v218467, v218469, v218471, v218473, v218475, v218477, v218479, v218481, v218483, v218485, v218487, v218489, v218491, v218493, v218495, v218497, v218499, v218501, v218503, v218505, v218507, v218509, v218511, v218513, v218515, v218517, v218519, v218521, v218523, v218525, v218527, v218529, v218531, v218533, v218535, v218537, v218539, v218541, v218543, v218545, v218547, v218549, v218551, v218553, v218555, v218557, v218559, v218561, v218563, v218565, v218567, v218569, v218571, v218573, v218575, v218577, v218579, v218581, v218583, v218585, v218587, v218589, v218591, v218593, v218595, v218597, v218599, v218601, v218603, v218605, v218607, v218609, v218611, v218613, v218615, v218617, v218619, v218621, v218623, v218625, v218627, v218629, v218631, v218633, v218635] : [u8; 527]
v218637 = add v28449, u32 179
...
// Repeat until item 431
v485994 = add v28449, u32 431 |
|
I can confirm that the SSA switches from using Prior to it we have this: v45841 = make_array [u8 0, u8 0, ..., u8 0, u8 0] : [u8; 527]
...
v46558 = make_array [v46447, v46449, ..., v43416, v43420, u8 0, u8 0, ..., u8 0, u8 0] : [u8; 527]
v46559 = if v36715 then v46558 else (if v36714) v45841
...
constrain v46566 == v46567, "ciphertext length mismatch"
v46568 = lt u32 0, v45839
v46569 = mul v46568, v36715
constrain v46569 == v36715, "Index out of bounds"
// read item 0
v46570 = array_get v45840, index u32 0 -> u8
v46571 = array_set v46559, index v46562, value v46570
// increment i
v46572 = add v46562, u32 1
v46573 = if v36715 then v46571 else (if v36714) v45841
// offset + 1
v46574 = add v46562, u32 1
v46575 = lt u32 1, v45839
v46576 = mul v46575, v36715
constrain v46576 == v36715, "Index out of bounds"
// read item 1
v46577 = array_get v45840, index u32 1 -> u8
v46578 = array_set v46573, index v46574, value v46577
// increment i
v46579 = add v46574, u32 1
v46580 = if v36715 then v46578 else (if v36714) v45841
// offset + 2
v46581 = add v46562, u32 2
v46582 = lt u32 2, v45839
v46583 = mul v46582, v36715
constrain v46583 == v36715, "Index out of bounds"
// read item 2
v46584 = array_get v45840, index u32 2 -> u8
v46585 = array_set v46580, index v46581, value v46584
|
* master: feat!: type-check trait default methods (#6645) feat: `--pedantic-solving` flag (#6716) feat!: update `aes128_encrypt` to return an array (#6973) fix: wrong module to lookup trait when using crate or super (#6974) fix: Start RC at 1 again (#6958) feat!: turn TypeIsMorePrivateThenItem into an error (#6953) fix: don't fail parsing macro if there are parser warnings (#6969) fix: error on missing function parameters (#6967)
TomAFrench
reviewed
Jan 8, 2025
test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr
Outdated
Show resolved
Hide resolved
5 tasks
|
With a statically known ...
enable_side_effects v62654
v62666 = mul v52946, v62652
enable_side_effects v52946
inc_rc v62665
// initialize encrypted_bytes
v62667 = make_array [v62664, u8 0, u8 0, ..., u8 0] : [u8; 527]
...
// re-initialize encrypted_bytes to contain the header
v62777 = make_array [v62664, ..., v59629, u8 0, ..., u8 0] : [u8; 527]
v62779 = eq u32 432, v62053
v62780 = cast v52946 as u32
v62781 = mul u32 432, v62780
v62782 = mul v62053, v62780
constrain v62781 == v62782, "ciphertext length mismatch"
v62783 = lt u32 0, v62053
v62784 = mul v62783, v52946
constrain v62784 == v52946, "Index out of bounds"
// read incoming_header_ciphertext[0]
v62785 = array_get v62054, index u32 0 -> u8
// re-initialize encrypted_bytes to contain the header, padding, then incoming_header_ciphertext[0]
v62786 = make_array [v62664, ..., v59629, u8 0, ..., u8 0, v62785, u8 0, ..., u8 0] : [u8; 527]
v62787 = lt u32 1, v62053
v62788 = mul v62787, v52946
constrain v62788 == v52946, "Index out of bounds"
// read incoming_header_ciphertext[1]
v62789 = array_get v62054, index u32 1 -> u8
// re-initialize encrypted_bytes to contain the header, padding, then incoming_header_ciphertext[0..=1]
v62790 = make_array [v62664, ..., v59629, u8 0, ..., u8 0, v62785, v62789, u8 0, ..., u8 0] : [u8; 527]
v62791 = lt u32 2, v62053
v62792 = mul v62791, v52946
constrain v62792 == v52946, "Index out of bounds"
// read incoming_header_ciphertext[2]
v62793 = array_get v62054, index u32 2 -> u8
... and so on, no looping copy ...With this change and #6983 applied the bytecode size is 1MB. |
aakoshh
force-pushed
the
6929-bytecode-blowup
branch
from
9a259d4 to
a48e557
Compare
|
@aakoshh Let's create an issue reminding us to revisit This program above should be solvable with a single memory block (edit: maybe not a single one but definitely not 19) which we read and write to directly (with the internal arrays just resulting in offsets to the indices we're reading/writing to) however we're currently:
We're then getting a total of 19 memory blocks being generated for something which should need a couple at most. @vezenovm's proposed changes to how we handle arrays should help here. |
TomAFrench
reviewed
Jan 9, 2025
test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr
Outdated
Show resolved
Hide resolved
|
|
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
AztecBot
added a commit
to AztecProtocol/aztec-packages
that referenced
this pull request
Jan 9, 2025
…eir trait in scope (noir-lang/noir#6901) feat(lsp): use trait method docs for trait impl method docs on hover (noir-lang/noir#7003) chore: turn on averaging for protocol circuits metrics in CI (noir-lang/noir#6999) feat(comptime): Implement to_be_bits and to_le_bits in the interpreter (noir-lang/noir#7008) chore: Add short circuit in ssa-gen for known if conditions (noir-lang/noir#7007) chore: Only resolved globals monomorphization (noir-lang/noir#7006) chore: Remove resolve_is_unconstrained pass (noir-lang/noir#7004) chore: require safety doc comment for unsafe instead of `//@safety` (noir-lang/noir#6992) fix: Reproduce and fix bytecode blowup (noir-lang/noir#6972) chore: mark casts as able to be deduplicated (noir-lang/noir#6996) fix: return trait impl method as FuncId if there's only one (noir-lang/noir#6989) chore(ci): fail properly in `external-repo-checks` (noir-lang/noir#6988) fix: allow multiple trait impls for the same trait as long as one is in scope (noir-lang/noir#6987) chore: Use DFG in SSA printer (noir-lang/noir#6986) chore!: Reserve `enum` and `match` keywords (noir-lang/noir#6961)
AztecBot
added a commit
to AztecProtocol/aztec-packages
that referenced
this pull request
Jan 10, 2025
…hem (noir-lang/noir#6913) feat!: require trait primitive functions/calls to have their trait in scope (noir-lang/noir#6901) feat(lsp): use trait method docs for trait impl method docs on hover (noir-lang/noir#7003) chore: turn on averaging for protocol circuits metrics in CI (noir-lang/noir#6999) feat(comptime): Implement to_be_bits and to_le_bits in the interpreter (noir-lang/noir#7008) chore: Add short circuit in ssa-gen for known if conditions (noir-lang/noir#7007) chore: Only resolved globals monomorphization (noir-lang/noir#7006) chore: Remove resolve_is_unconstrained pass (noir-lang/noir#7004) chore: require safety doc comment for unsafe instead of `//@safety` (noir-lang/noir#6992) fix: Reproduce and fix bytecode blowup (noir-lang/noir#6972) chore: mark casts as able to be deduplicated (noir-lang/noir#6996) fix: return trait impl method as FuncId if there's only one (noir-lang/noir#6989) chore(ci): fail properly in `external-repo-checks` (noir-lang/noir#6988) fix: allow multiple trait impls for the same trait as long as one is in scope (noir-lang/noir#6987) chore: Use DFG in SSA printer (noir-lang/noir#6986) chore!: Reserve `enum` and `match` keywords (noir-lang/noir#6961)
AztecBot
added a commit
to AztecProtocol/aztec-packages
that referenced
this pull request
Jan 10, 2025
…ng/noir#6913) feat!: require trait primitive functions/calls to have their trait in scope (noir-lang/noir#6901) feat(lsp): use trait method docs for trait impl method docs on hover (noir-lang/noir#7003) chore: turn on averaging for protocol circuits metrics in CI (noir-lang/noir#6999) feat(comptime): Implement to_be_bits and to_le_bits in the interpreter (noir-lang/noir#7008) chore: Add short circuit in ssa-gen for known if conditions (noir-lang/noir#7007) chore: Only resolved globals monomorphization (noir-lang/noir#7006) chore: Remove resolve_is_unconstrained pass (noir-lang/noir#7004) chore: require safety doc comment for unsafe instead of `//@safety` (noir-lang/noir#6992) fix: Reproduce and fix bytecode blowup (noir-lang/noir#6972) chore: mark casts as able to be deduplicated (noir-lang/noir#6996) fix: return trait impl method as FuncId if there's only one (noir-lang/noir#6989) chore(ci): fail properly in `external-repo-checks` (noir-lang/noir#6988) fix: allow multiple trait impls for the same trait as long as one is in scope (noir-lang/noir#6987) chore: Use DFG in SSA printer (noir-lang/noir#6986) chore!: Reserve `enum` and `match` keywords (noir-lang/noir#6961)
AztecBot
added a commit
to AztecProtocol/aztec-packages
that referenced
this pull request
Jan 10, 2025
chore: delete docs for versions which aren't used (noir-lang/noir#7020) feat!: Disable mocks in `execute` (noir-lang/noir#6869) chore(docs): backport 1.0.0-beta.0 doc fixes (noir-lang/noir#7014) feat(cli): Add CLI option to filter by contract function name (noir-lang/noir#7018) chore: Add more Field use info (noir-lang/noir#7019) fix: let static_assert fail with the provided message (noir-lang/noir#7005) chore: mark `aztec-nr` as expected to compile (noir-lang/noir#7015) chore: clarity fix in docs (noir-lang/noir#7016) fix: require generic trait impls to be in scope to call them (noir-lang/noir#6913) feat!: require trait primitive functions/calls to have their trait in scope (noir-lang/noir#6901) feat(lsp): use trait method docs for trait impl method docs on hover (noir-lang/noir#7003) chore: turn on averaging for protocol circuits metrics in CI (noir-lang/noir#6999) feat(comptime): Implement to_be_bits and to_le_bits in the interpreter (noir-lang/noir#7008) chore: Add short circuit in ssa-gen for known if conditions (noir-lang/noir#7007) chore: Only resolved globals monomorphization (noir-lang/noir#7006) chore: Remove resolve_is_unconstrained pass (noir-lang/noir#7004) chore: require safety doc comment for unsafe instead of `//@safety` (noir-lang/noir#6992) fix: Reproduce and fix bytecode blowup (noir-lang/noir#6972) chore: mark casts as able to be deduplicated (noir-lang/noir#6996) fix: return trait impl method as FuncId if there's only one (noir-lang/noir#6989) chore(ci): fail properly in `external-repo-checks` (noir-lang/noir#6988) fix: allow multiple trait impls for the same trait as long as one is in scope (noir-lang/noir#6987) chore: Use DFG in SSA printer (noir-lang/noir#6986) chore!: Reserve `enum` and `match` keywords (noir-lang/noir#6961)
AztecBot
added a commit
to AztecProtocol/aztec-packages
that referenced
this pull request
Jan 10, 2025
chore: delete docs for versions which aren't used (noir-lang/noir#7020) feat!: Disable mocks in `execute` (noir-lang/noir#6869) chore(docs): backport 1.0.0-beta.0 doc fixes (noir-lang/noir#7014) feat(cli): Add CLI option to filter by contract function name (noir-lang/noir#7018) chore: Add more Field use info (noir-lang/noir#7019) fix: let static_assert fail with the provided message (noir-lang/noir#7005) chore: mark `aztec-nr` as expected to compile (noir-lang/noir#7015) chore: clarity fix in docs (noir-lang/noir#7016) fix: require generic trait impls to be in scope to call them (noir-lang/noir#6913) feat!: require trait primitive functions/calls to have their trait in scope (noir-lang/noir#6901) feat(lsp): use trait method docs for trait impl method docs on hover (noir-lang/noir#7003) chore: turn on averaging for protocol circuits metrics in CI (noir-lang/noir#6999) feat(comptime): Implement to_be_bits and to_le_bits in the interpreter (noir-lang/noir#7008) chore: Add short circuit in ssa-gen for known if conditions (noir-lang/noir#7007) chore: Only resolved globals monomorphization (noir-lang/noir#7006) chore: Remove resolve_is_unconstrained pass (noir-lang/noir#7004) chore: require safety doc comment for unsafe instead of `//@safety` (noir-lang/noir#6992) fix: Reproduce and fix bytecode blowup (noir-lang/noir#6972) chore: mark casts as able to be deduplicated (noir-lang/noir#6996) fix: return trait impl method as FuncId if there's only one (noir-lang/noir#6989) chore(ci): fail properly in `external-repo-checks` (noir-lang/noir#6988) fix: allow multiple trait impls for the same trait as long as one is in scope (noir-lang/noir#6987) chore: Use DFG in SSA printer (noir-lang/noir#6986) chore!: Reserve `enum` and `match` keywords (noir-lang/noir#6961)
AztecBot
added a commit
to AztecProtocol/aztec-packages
that referenced
this pull request
Jan 11, 2025
…ang/noir#6985) feat!: disallow calling unconstrained functions outside of `unsafe` blocks and passing unconstrained functions in place of constrained functions (noir-lang/noir#6938) chore: simplify a couple of enum variants (noir-lang/noir#7025) chore: disallow inserting ACIR-only instructions into brillig functions (noir-lang/noir#7017) feat: unchecked math operations in SSA (noir-lang/noir#7011) chore: delete docs for versions which aren't used (noir-lang/noir#7020) feat!: Disable mocks in `execute` (noir-lang/noir#6869) chore(docs): backport 1.0.0-beta.0 doc fixes (noir-lang/noir#7014) feat(cli): Add CLI option to filter by contract function name (noir-lang/noir#7018) chore: Add more Field use info (noir-lang/noir#7019) fix: let static_assert fail with the provided message (noir-lang/noir#7005) chore: mark `aztec-nr` as expected to compile (noir-lang/noir#7015) chore: clarity fix in docs (noir-lang/noir#7016) fix: require generic trait impls to be in scope to call them (noir-lang/noir#6913) feat!: require trait primitive functions/calls to have their trait in scope (noir-lang/noir#6901) feat(lsp): use trait method docs for trait impl method docs on hover (noir-lang/noir#7003) chore: turn on averaging for protocol circuits metrics in CI (noir-lang/noir#6999) feat(comptime): Implement to_be_bits and to_le_bits in the interpreter (noir-lang/noir#7008) chore: Add short circuit in ssa-gen for known if conditions (noir-lang/noir#7007) chore: Only resolved globals monomorphization (noir-lang/noir#7006) chore: Remove resolve_is_unconstrained pass (noir-lang/noir#7004) chore: require safety doc comment for unsafe instead of `//@safety` (noir-lang/noir#6992) fix: Reproduce and fix bytecode blowup (noir-lang/noir#6972) chore: mark casts as able to be deduplicated (noir-lang/noir#6996) fix: return trait impl method as FuncId if there's only one (noir-lang/noir#6989) chore(ci): fail properly in `external-repo-checks` (noir-lang/noir#6988) fix: allow multiple trait impls for the same trait as long as one is in scope (noir-lang/noir#6987) chore: Use DFG in SSA printer (noir-lang/noir#6986) chore!: Reserve `enum` and `match` keywords (noir-lang/noir#6961)
AztecBot
added a commit
to AztecProtocol/aztec-packages
that referenced
this pull request
Jan 11, 2025
feat!: disallow calling unconstrained functions outside of `unsafe` blocks and passing unconstrained functions in place of constrained functions (noir-lang/noir#6938) chore: simplify a couple of enum variants (noir-lang/noir#7025) chore: disallow inserting ACIR-only instructions into brillig functions (noir-lang/noir#7017) feat: unchecked math operations in SSA (noir-lang/noir#7011) chore: delete docs for versions which aren't used (noir-lang/noir#7020) feat!: Disable mocks in `execute` (noir-lang/noir#6869) chore(docs): backport 1.0.0-beta.0 doc fixes (noir-lang/noir#7014) feat(cli): Add CLI option to filter by contract function name (noir-lang/noir#7018) chore: Add more Field use info (noir-lang/noir#7019) fix: let static_assert fail with the provided message (noir-lang/noir#7005) chore: mark `aztec-nr` as expected to compile (noir-lang/noir#7015) chore: clarity fix in docs (noir-lang/noir#7016) fix: require generic trait impls to be in scope to call them (noir-lang/noir#6913) feat!: require trait primitive functions/calls to have their trait in scope (noir-lang/noir#6901) feat(lsp): use trait method docs for trait impl method docs on hover (noir-lang/noir#7003) chore: turn on averaging for protocol circuits metrics in CI (noir-lang/noir#6999) feat(comptime): Implement to_be_bits and to_le_bits in the interpreter (noir-lang/noir#7008) chore: Add short circuit in ssa-gen for known if conditions (noir-lang/noir#7007) chore: Only resolved globals monomorphization (noir-lang/noir#7006) chore: Remove resolve_is_unconstrained pass (noir-lang/noir#7004) chore: require safety doc comment for unsafe instead of `//@safety` (noir-lang/noir#6992) fix: Reproduce and fix bytecode blowup (noir-lang/noir#6972) chore: mark casts as able to be deduplicated (noir-lang/noir#6996) fix: return trait impl method as FuncId if there's only one (noir-lang/noir#6989) chore(ci): fail properly in `external-repo-checks` (noir-lang/noir#6988) fix: allow multiple trait impls for the same trait as long as one is in scope (noir-lang/noir#6987) chore: Use DFG in SSA printer (noir-lang/noir#6986) chore!: Reserve `enum` and `match` keywords (noir-lang/noir#6961)
TomAFrench
added a commit
to AztecProtocol/aztec-packages
that referenced
this pull request
Jan 11, 2025
Automated pull of development from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE feat!: require trait primitive functions/calls to have their trait in scope (noir-lang/noir#6901) feat(lsp): use trait method docs for trait impl method docs on hover (noir-lang/noir#7003) chore: turn on averaging for protocol circuits metrics in CI (noir-lang/noir#6999) feat(comptime): Implement to_be_bits and to_le_bits in the interpreter (noir-lang/noir#7008) chore: Add short circuit in ssa-gen for known if conditions (noir-lang/noir#7007) chore: Only resolved globals monomorphization (noir-lang/noir#7006) chore: Remove resolve_is_unconstrained pass (noir-lang/noir#7004) chore: require safety doc comment for unsafe instead of `//@safety` (noir-lang/noir#6992) fix: Reproduce and fix bytecode blowup (noir-lang/noir#6972) chore: mark casts as able to be deduplicated (noir-lang/noir#6996) fix: return trait impl method as FuncId if there's only one (noir-lang/noir#6989) chore(ci): fail properly in `external-repo-checks` (noir-lang/noir#6988) fix: allow multiple trait impls for the same trait as long as one is in scope (noir-lang/noir#6987) chore: Use DFG in SSA printer (noir-lang/noir#6986) chore!: Reserve `enum` and `match` keywords (noir-lang/noir#6961) END_COMMIT_OVERRIDE --------- Co-authored-by: Tom French <[email protected]>
AztecBot
added a commit
to AztecProtocol/aztec-nr
that referenced
this pull request
Jan 12, 2025
Automated pull of development from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE feat!: require trait primitive functions/calls to have their trait in scope (noir-lang/noir#6901) feat(lsp): use trait method docs for trait impl method docs on hover (noir-lang/noir#7003) chore: turn on averaging for protocol circuits metrics in CI (noir-lang/noir#6999) feat(comptime): Implement to_be_bits and to_le_bits in the interpreter (noir-lang/noir#7008) chore: Add short circuit in ssa-gen for known if conditions (noir-lang/noir#7007) chore: Only resolved globals monomorphization (noir-lang/noir#7006) chore: Remove resolve_is_unconstrained pass (noir-lang/noir#7004) chore: require safety doc comment for unsafe instead of `//@safety` (noir-lang/noir#6992) fix: Reproduce and fix bytecode blowup (noir-lang/noir#6972) chore: mark casts as able to be deduplicated (noir-lang/noir#6996) fix: return trait impl method as FuncId if there's only one (noir-lang/noir#6989) chore(ci): fail properly in `external-repo-checks` (noir-lang/noir#6988) fix: allow multiple trait impls for the same trait as long as one is in scope (noir-lang/noir#6987) chore: Use DFG in SSA printer (noir-lang/noir#6986) chore!: Reserve `enum` and `match` keywords (noir-lang/noir#6961) END_COMMIT_OVERRIDE --------- Co-authored-by: Tom French <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Problem*
Fixes #6929
Summary*
Reproduces the bytecode blowup when switching from unconstrained to constrained encrypted note emission in AztecProtocol/aztec-packages#10535
Fixes the issue by doing another mem2reg pass before flattening the CFG.
Additional Context
See the flame graph in the comments below for what we're trying to reproduce. It looks like this line results in lots of opcodes/gates coming from
AssertZero,MemOpandRangeinstructions.NB the size blowup goes away if we use
let size = M - 32 - HEADER_SIZE - OVERHEAD_PADDING;instead oflet size = M - offset;, but that's inaztec-packages, it doesn't work here. We created https://github.com/noir-lang/vscode-noir/issues/92 to capture that.Examining the original SSA (see the comments in this PR 👇) revealed that the source of RANGE checks was a bug with unsigned multiplications, which was fixed by #6983 , but it doesn't make the array-copy phenomenon go away.
The item-by-item array copying is related to how
Remove IfElsepass handles conditional variables and arrays. The bytecode size in this example depends on where we place theflagvariable (not part of the original program).Testing
To see gate counts:
To see the final SSA:
The fix
Looking at the SSA with smaller numbers (an array of size 15 instead of 527 items) just before the
Remove IfElselooked like this:After Inlining (2nd)
What we see there is that for each field, we make a copy of the array, then either use it or discard it and keep the initial value
v5depending on the conditional variablev3. After this, inRemove IfElse, theValueMergerwould take all the instructions likev75 = if v3 then v73 else (if v21) v5and merge the arrays item-by-item, multiplying each item with the conditional, before making a new array out of them. This results in a quadratic blowup of instructions.Instead what we'd ideally do is modify a copy of the array with each item, and then at the end have a single if-then-else to merge it with
v5, depending onv3.Now take a look at the step before
Flattening:After Simplifying (2nd)
@TomAFrench 's insight is that to achieve what we outlined above (the single merge at the end), we just have to get rid of the redundant store and load instructions:
Adding another mem2reg before flattening indeed had the desired effect 🎉
After Flattening
Documentation*
Check one:
PR Checklist*
cargo fmton default settings.