From ba727c68c2d5972d1562f30ebade120d4622a71f Mon Sep 17 00:00:00 2001 From: Leo Alt Date: Thu, 5 Oct 2023 12:31:19 +0200 Subject: [PATCH 01/15] adjust reamde --- README.md | 35 +++++++++++++++++------------------ book/src/powdr_wires.png | Bin 0 -> 20312 bytes 2 files changed, 17 insertions(+), 18 deletions(-) create mode 100644 book/src/powdr_wires.png diff --git a/README.md b/README.md index 430b0032a9..a64694c60f 100644 --- a/README.md +++ b/README.md @@ -1,34 +1,33 @@ +

+ +

+ # powdr [![Matrix Chat](https://img.shields.io/badge/Matrix%20-chat-brightgreen?style=plastic&logo=matrix)](https://matrix.to/#/#powdr:matrix.org) [![Twitter Follow](https://img.shields.io/twitter/follow/powdr_labs?style=plastic&logo=twitter)](https://twitter.com/powdr_labs) -*powdr* is an extended polynomial identity (PIL) and zk-focused assembly (zkASM) -language written in Rust, focused on modularity and excellent developer experience. - -WARNING: This is a proof-of-concept implementation. It is missing many internal checks. DO NOT USE FOR PRODUCTION! +> WARNING: This codebase is experimental and has not been audited. DO NOT USE FOR PRODUCTION! -## Basic Concept +For detailed documentation please visit [the powdr book](https://docs.powdr.org/). *powdr* is a toolkit that helps build zkVMs and similar proof frameworks. It has two main components: -- A polynomial identity language that allows you to define polynomial constraints, lookups, etc. -- An extensible assembly language to perform dynamic executions. +- powdr-asm: an extensible assembly IR language to perform dynamic executions. +- powdr-PIL: a low level constraint language that allows you to define arithmetic constraints, lookups, etc. Both frontend and backend are highly flexible. As an example, *powdr* contains a frontend that enables you to write code in (no-std) Rust, -which is compiled to RISCV, then to powdr-asm and finally to PIL. +which is compiled to RISCV, then to powdr-asm and finally to powdr-PIL. *powdr*-pil can be used to generate proofs using multiple backends, such as: -- Halo2 -- eSTARKs: *powdr*-pil is fully compatible with the eSTARKS backend from Polygon Hermez, - although not yet fully integrated in an automatic way. -- Nova: ongoing work. -- other STARKs: maybe? +- Halo2: via [polyexen](https://github.com/Dhole/polyexen) and [snark-verifer](https://github.com/privacy-scaling-explorations/snark-verifier/) +- eSTARK: via [Eigen's starky](https://github.com/0xEigenLabs/eigen-zkvm/) +- SuperNova: ongoing work (https://github.com/powdr-labs/powdr/pull/453) All stages are fully automatic, which means you do not need to write any additional code for witness generation besides your Rust code. All witnesses @@ -37,7 +36,7 @@ inferred, *powdr* can ensure that the system is not underconstrained, i.e. there are no additional unwanted witnesses. All artifacts from the compilation pipeline are human-readable. This means you -can inspect the RISCV assembly files, the powdr-asm, and the PIL file. +can inspect the RISCV assembly files, the powdr-asm IR, and the compiled PIL file. The assembly language is designed to be extensible. This means that it does not have a single native instruction. Instead, all instructions are user-defined and because of that, @@ -45,10 +44,10 @@ it is easy to adapt *powdr* assembly to any VM. ### Notes on Efficiency -Currently, the code is quite wasteful. It generates many unnecessary columns. -The idea is to first see if automatic witness generation is possible in general. -If this is confirmed, various optimizer stages will be built to reduce the -column (and row) count automatically. +The current focus of the project is VM support and developer experience. The +compiler generates many unnecessary columns. We will soon start writing +optimizer steps that should bring performance closer to existing production +systems. ### Project structure diff --git a/book/src/powdr_wires.png b/book/src/powdr_wires.png new file mode 100644 index 0000000000000000000000000000000000000000..2772f906e31420cdfd55cc4d03c4ac033ce4303a GIT binary patch literal 20312 zcmb4qcR1T`)Hg*@HCi=VyDf?ut)eznwW*P6ZM8>Yr1su5irRaRqBe;bQKPl@7Lr=A zH?es_f6x2;{kkrfxbwZoIp=f6ea`uYzE_naz4Pb}4h{~fg8W+z92~p_92{I>qTAS! zGx%-=_7AbW{3kFD4hiM;FD_0>8ZCAZ7px)o2B)l_eii!6v??U0S=Dp zrovlkEmz#lSucA%?G)6}@(jrSM`HG)l(D-Uj_>-un{V`quq0{OmJ9I+JS!G!sO9l5 zdkrXh-4o3J`T2H%JX?ICiUv!C8<#fc?c{D9(P1rIdkqfJ%K#$-JqP)FeOjC5T&f9K z6H7&7@@?pt=*903YRp-`eRzvn0QWaR9U6vaS3n0nO|vL9ZfYqPyeBcgnvPsDm&N4p zv8Vn2{hsFM)j4XSMocT$-n176yar1?56Yz{g9N%HxaRD#T;bur=KK(oL!2z9aQ2qAPn~^N; zi)liFRnPQe5L}Y-Dvpordia;YOHSUi>_*C6HDat>u|Ibo8q;6@^%XZ0G?lFJk-X&i zZ=gw8Ve0%U?4WyJ(JAv95I8uNN_}eD?MLN`yftxC7XRiM(C?Ieb)?$cdX{YG`Sss6 zQiT3%b!qWi(_=JrCG*t(El5_hKSKBT@{r6x=N|Vy+gILK7ZSH_))mP~gzi;wA2~A) zy2utS_sR)7;Ldm%#QfBAh(DmN`tKpl_`~TAoe!fYh0FPbHAVlphkNm7+>G+q{#+py zWiL&>+ zXe5J?;UTx8CicyDWRLI>Hxi-FcC>{ zUaz-i|9Dh)%4{H*$*Y)o?;j+HR)ru5kDxEsEj-)<|LyFX-F!48yTA2$Bm1{BzC*ac zjd*-Rm&ZfxdYaM+Pn%W6BD-&pVU#m=8#2ZiU8UT``jGkGS&XWHJ+;Apz-nvn`p0;0 zX$h|nr2Jf{LdCQI;~R4#T}m1CyMF{mq!&o*EyjpQHa#Ym z`fn(nTG~^+>7|$KeScig^}@cDLs+cI`&0bmy$K7q^vvUbtkB?9`M=NH!2kQq^-X%_ zbBf6Ibmn+viUB3SmVCE=o$Nn(MvE9z7}9W`Otuvf{V%$sIaLDc65Oy9$!houI$h6{ zvR;{hnh!I!m;Su|7bp7GX)XDkoW&Gk6l^Cu?8J2(f0tC@A?H6yw~wICR&q?Movqy| z+H2!R9<_6i|2yS4q`0u+O{9V>iLWl3)Y6u;||KWq# z;b8BBDr70;q@rR!3(l=+t%_CKr%OctHhSxzn@Aw-$%fe}b~mekX7azM{X7ccDIuQ^ zUwF{Y{SQ>k_YRJr{h`*AGFT=P&y&0~x754=R>VwM=&Cch!}n^ouZgC2|3meYh1<0N zvo5W+a8dFCpY^Jy|2Y0+=*xQqt%|Z;$Ul_OiU|GaTvr&^rGcgYEsHnt#jCC}*llCuHMzA-&_AC4 zl3JaRcqR`R&zJaGuISvzUI3N(Cp11jkXPL@w}6~j#aL=}pGO|jR)XCW?O|EVELPxll;JogQeUPb3bzAaSuk=R4!2n&cgGm+$9a5GSi>*t_Cj8ytz0G~p6 z){BchPgm4o8XN$8TFMTxOdc`MD{}>3Vxy=l`ZnCV;g^@=!kQP5^0tKw9MdpIn7O07lNgw64&R8hSBJ6q`3EcZ#yg%<(xDn z2{beHpusHBCNmsM>HN#VXXv&V9OC`Og-?iX&x^SRA+_$LE~ZOWc*+zUe2|>D4f5tg zPRQ4?0Ka*JARq_Y1xo&2*)PeSD!Z~zX^AdI?Dfu?azp#{mtur!n;@vavqgWOYH}yt zF_9MY`lE#qs=;WN%qJp!d|qFXN)_fXXYBGfl(dDAoD?Zz zBDYJr(H=sPB+<-&ZQ;9C>l@A><@}4 zDz!3G$rth4_&*87oO{oq0Kkj8XjTvA0`B_?gS73r9mI>YE)dz-{YHMYVwq8yOuwV@ zJB>i4V02X`aDZ*@k$E)S`*q-eLL=Ex7y z5}hQYOG7?~#vMZRI4_8QlRvAUA3vY^Aj1lahyiSmY{uthG};l2^?U;p++S4}yIZ-Z!4L{W6Uh|HlrBuo`51Zu~BuulHq6frFnnFhD?rsmyn(qokC z$PjPJs!cmNC`43S&CX>lC73-9ziUND-|9*i#I(}rBDJO9!s!h#S3-Tb+xSMaB`DTF z)rQD!u{E-oCrNVT;pSN)(GqQqYJh8?RHOrC?GH)UBlM@UlZmJdWDbBK_se1lZ=YE; za`@@(-OkEfUj2K9>8{7fzE29t6^}rL`PF7q(f9r!Tvz4XXH^onVop+qPf0kPn!nOM z*X`;o-l)>xEEgBg#84nR0o`7;8#ekyYn#<~-P@SI(6v9o0xkB3tQUCyPuST^Adb%1 z3r@KxxY!f8Uh3YWty`;$p|Jbn5|ug}>3P)Wrr<|CW*^So6WmVu%B*C*3E!cI%oU7> z@{I=0cK;}H0-vp80$l9qnV`&|DaqK2YPXDfktLNc3(Nv`K?|L)L9=xX1<>?!&!7x z@3)79@FBakW)FUqxv6!g8O=|{#+4c!XY{J9_HMMf*zt5Gc^L6f09STV-*~BSx_HYUAZI+~9VD1ye2a+4BwVNR#i`+EaLlYUdPBjRAhh zqcYT_4tYq;@9+?3;IV>}cS=4)<}IyV%g7nYLcy&R8s!>0dhgY=(qNnK>*PXq)NEFy z@fXz+6)qw1^^2hflFo~)TNODL7ew>-ZV3B|${@K{54R7c0Nnv8mGWX=6X6p8A+9dN@@#5Hq z0(jT;k5*Utk|RNaT^84ncP%J2O=5QNarBeY6vz)bOF&2TB-rhpS@{&R1Tlk8)7O4} zgLnEBzaKt)(O~y_Y{&F*1T-iA`B_%nj*Ph925~neAJI=l!}w8>N~;Uiu=1L^8?A7`$Dz9Xcv)|r#WR89nDtTAO+rWrF- z?)82Ug{snh#oNvpi6Jd~ltB@=ORd-e_^kz18cwC!KQwD;|2nRktdw6e!vX5MoOPYn z?RB3L&23bQFr|BUt=`|Mfx4qlYKl#KH(qFPahOUVRDVPC6^t7`#Fg`qme6J=LLCbp z!JkfT)Wc*FQEqQg30NnqAVX`HTWpHd5^iXqcvXHRZcL6KzMsh^(zV_udid81%(B|^ z$*#Nk9kX1n@$FqAKYFXdGFkCAB_g+3ALUsQ#txlKhst zSk8gw0-3*VN9oI%;lcFfWSK+sLtYm%GqOZKDH8d|dkK-fu|#xlUpfi??xnro%m_5o z^SOu0X&yvBIR=J}GxkqD70-3a(Qf;eenFiUh=BYtjpL59jm_J|OX#MHna0w0xrMyRx%sBV6V*k;Tn2U99 zyDyP3=|rj&+f1T^AHwT>4yF@myG04F?z+9v>8r#Zryus63!+SBW=bBbFB9$>d#x zGA9Mm11V)`xlB>|{#z&2J{1WH0XJqhDr<=j>+>*dh|V zd^S3j?mkS$5&VkyFpYH`SPUu*s)V}-esDVAgx`%~_Cplddz$d-Dsz3o+{} zh=?iUpU?qn7oZDQpDVMpx-$KSzTt0ae1Dt`e@07(!Os_mZ3HI`DvDElN>3MJ&j;&C zMGDci_f=|$yB?X(Lpwun!Pq(Fm^+?*5s_`Wr#UvnEjou#J%vc8Z8gtT*z9gP@LGhts+OUAlR*r+_MJx0Cq;k5dBg9%cu0Td zQc~jhfvag|8z>olSDiv_MCSut=jJ!Op^V4TCQ0u95X&Quy<5?v4uJm0_An`se=W!H z^LCfS(9MT%=DU`3x`DeA`{Jsa&c*v^54+(p;akow4>wEP8fMpr4c$ur>I|oDKIC~>(ywNi z$Li%t@#n0U47^`_KeJI(`#772NQ3K#@{iAd2EGVv-f!wr1<-7W=<_`8kej}7_r`0r zH&#K!n1syPqpQ~fu|fah4f*AIGjw;_@bXCTaxL>De0_K_hR3n1Hjh6GxIyuU@Glyw z#eppAM=q?Q*}^!dM?$(F1U7~PsZSn^jCK5~!@m}GTg5-iFd0BP5BVE?SRI-<_o6?C zW3~Cb@X~f}u!a4zS|Imox$;P|Z~@T*-n@lrwqUQy(`|;BNGP|IBfYq1@k(+@HsKkJ z_Klitc)l<(kY0RmFO0eN5BE(JR&!Bv?P=shosB+Do(L(m7{5AL*X8dk$g}gp?9LjV zFkO}G9r)NMaIBg{@<}j5^k)6V2z|Q&lj0x!gIcIV{T~?!_WmeTmuD_t|@aUXBoLJ;sSQ zcyB}M5j9el@z`0-LA2&u?C45evq!#mrl(ksRPuvL>GRNzs&$>w3kTBq?>fwqj;CZo z-v)WT{>k_Db;M%=8hnl*M(Qox&Add)(U|FOC6lk010U%OGlAf4Luqo7=-_SFClTZy zXORmvC57)K3_Ag`hDMf%6__zdk%wO5JiU>57wgp{U0piFzsmVb7S`Kb&EPv8$Vlp3 zyO<~a$d!AjW^|}em+B7n-EXtSy;^PCUzexS6F%$4uOiS#7pB!5tD1)HUtZ8#DtXgt z2JYgL95F@_icF7b{vBFI(gSu)?b~L0XXhh|d5G=<@85vPdrkXBtv_N_J-bG+XpC^X zsf%$;<~p-hTiX5B+&RNumt~-yV|BSO6-iq;A$1ew044oA#qRwH`8&S9c4u>qgYqX; zA!h~!`mL#V$boS2LRT$i`{AYn$#wK{nqAJlc28%FJwu!gHf!A8aIrI+O5*QFG7)5$ zuvxI)N@Lco)h0Sw+>15u5Y%l)(JyLzABYxY-ZZ$oPZF=8eO)q(T^%PWzn?V0RGq|< zv1|f~7@$12`$7#+c6hK059a7x?F-v$eUq4S1|_&tdQ^Xz{ES^WuZ&gyL#xxAzKQ~L zB2O?Hk?kP|KOIgpvZeSF66kz*P_gN!I2y*RUc(C!@qQQmY?oQJmeuHrk!W!R(0t?E z)ymk(RJm2=_p;UP*IS#Loyu41?I&U+#8K-X%y^=?gj4&vhleZ-GVVnXu+kPMdgo2l zlIZ|yPMU@#zsKgshF#!Mwn7M1-89$9k=WoU`gI5Pj*(lkwuV02ILpAt-^E0Vd$_mI zZx48$j%-dT=C?aAdN?j;DP#Hr`cg-REkImc`3Wk_3+IB;aPP&MSqZMIzq6?p1b8Mg zb*9yj_1gDWfpIFU`!T0mUr^&K4P-QGWxz88JIbe7i#qR7{4PtbzW7vCas;rFn>a?H z$7w%YlOlp%+S5*}bEs7MMiDc`oc|^YF85-s$iBZH(Rh$g&3!TpuJ(-SPc+C5Y(S1O zTPjg)T7HOGs73x>yQmzqoY3vD zYgP1eoMbM0>G+~#)HAt14IWNv(O@yJb>Y<2?>$G-1j*Z8YB>}7u%U%@Cc@A>I%7|K zMcy9*rDg)BGdWkBru%r9S!GVq&b|-dA{o75&uEGLU5~J$YL_bb zA@sTU^lb$t6i6}fHN}i~_tHi@O$8+w#bgIDJZ8pUrmpRVvCa}LJjv;k)`&wG4t5uD ze4>ufdRIkYc!pOZpQRQzQvmRKW&6=}zOt7AyYReU8syC@{>~ex8ryO-dPlnP3d7is z`6AsUqMyK0wM;#*NDKyX+|dZ-lX#rJP&^NCnI7wt)yc*N#zVTdzNKvUJd9WO(w_*p z9I^KJ_Tg}@8h04dHr23$zl(h9m*(VcX+HQ*Iq;+#ee{TYOl)RLNhV^jvEqiR!ah%+ zONHTamgV;b^voMGv3%_5zwGN4FOJ6tim4^sv9p43()xMAqpQcpL&)tC@IxW6Pt$zt z>38DO=UBcTsf~V+XO^3Je`7)L7>Y>dN`tG@GY#$;F=1D7UMFxG!VlLtfOU7)VuP&N zQWnIenCQZE=65o95u;m8F*){7S44}JuOe<(Am!oi#)WczbLtyXZCpk1hcc_n+2SV? zlOW|rO}W*oBJt8HX(_;f?>#p5zy^Y#k2jS0&3>jF-hP!F4gbt2sX!#xeDB{xFm~ok zZeI5o4q}a#^6ucGjkJ4r?@D{_5G_^&Pv6+^A4RBwpIw1pgwlZS-ji)pd=H5hh>fA5 zCKtymNu1J1)VP^6rztrowfl9O;_V=22{C-fyVW;NanOp0Y13XXLbBD>fAVlaN1+su za=6fD90E-apQAquFASNr(+i&&pfos*WR~#6-|I5h9j%3e`Mp14?>|p%8nj=t(96V8 zJq;sXu!!$u04MZL9WLyd!OR&msP0D4A<;!u%z(I+Z^a@;4RYQ7aOSU3kr!tYm<=mU zRwqAC%*BzCS<7VVNmRl<6RJ*Ne+G4e*aLFQ5<}R#lQp`)TLfqwDLH9r*t&m$&`K>* zn3_}6vb#TLXs^<(dR>5_1cBtIv;{#qW3>5zIDDdp! z(<>HFSzngN5$qBAlHR8wr$4tt)Bmnum?RG)4eAdY>z~pR<$vkdYoDRX3mZ0&t{Et`eb#`dZW%T#Zyj*ZY&l|#UcGBGJqV#!OB`*|*;I>VxG+akM4C=)!dVz*a zUUrgZ^?PwoGrH>GT9w$>@}5>=v3XkYDYUKVWP+y9_1VMkXa*K6Bas4CZHX*)bF`h5~e< zZG2XMj@QhAPk>k*`5;opi8d8p|4mrWU{5)2p~POahzbAT-M?Rb?o|*m-W}}VOt*4R zTI_^qG1m;y-^6a({{harqJcPbHIab|dm#olPpXWIr<_ASjqLYSUZ)X+>X$6$d?x;2 zvq+^98k!{Ng&Z6XSt2j=JyP@IyaMphI}n5&9Uz~ zf%bU}#$p7mejw+RQ;p;Il{=9k7C<7kEFqahx&2*iva$BD{5Q#&Mz^Jv>Ox*LnPSzVFjnVC`5_%}b^`=Tx9 zN5rNP$15>eqK=Qu5rxXHjb{lQA!2wqMnV(8TQD6Cm+029KIJc(nZCLnH1$yAYN}MizRLZ+sb_*`nt3s2BscO1(Y^_k3o)I2&#r&poZ;ir_?yNS~ZlW z8k2oAuizeb0ga?Pf{l_A0JJHA(Tr^AO2(@uS2ZoFM)jNghSGV4xtpt4;Ox45j@ zsBdj_YZyNzF&o{e=kTcNCNxGNh%bih>?vk8o1H#wgesr8=)2NTjzcf1PRGh$0-2<& zsl2oHk#R74yYVo_nV&Tte&uXMcaYDzZajQP?42@j7!sEwW&L-&deEMc$IJ2+|6>px z`e>o}y2$zqpCuwHRCl0+0PI`Kw1vON+u)l#Bf5jP=zl~r_U8Q(Mm6bNhmY{8kv{2; zK}F^r$jVuZsLPR{>xlbuftY{*w5?-n$jxhut1 zTwy0K_T%g_n7$Wezx+^2W9sm)V%Jha=?Z#&R;#5v4xW5YH=tm5b30o)H^&7SZ#+J-z#`LVKe;g7mHm zfG-9ftocI|Lz}X4w;d%d353Q>UA`|9Y{$FVA{rUuzW6d{U@%OPR0rILYen;%BI<@Y53qn@=u;t^=tr z>@1!VC0DXo1Di)bR0a%MwK>1*4!YWEU2?g+a^V&COzKS#yKXXxod^neZ*J|Uf8w@3 z#xI@U1dR9kQ@hS(*Y8EGHM}{{{qDW&1nVw8UHYX9j#N43&YZJu)l+B=G$#0;IxP3m z^paA4uWc1i*9LrgUtFfrSiZSln{C^zwdAsWPnC%95f+Q@M-mR>%2~*#`!8~cBX||< zzbCB!E!hgU^UrgoE6{-(drzlAOqoO-PyJ1w9=1yA7LDzn9&&^CThnMu;KWGIk6ftw z9MF|Fi80lMzkUG-4u38!)eO{~87mox!^b(z0ul)UcJj&zQA#>DL2guGg^t0iY|m`K zGv3BrnnjXNn}TuMDG|szxjKw?{+hmexr9D7jzHk<3js1eFQWQXHca2av^XX=A|1hX+W|HHzeX*<;mKHPu=@zHqdaV{sc-ik)5b_*tk|+ zc}Xt?)d8tdgH+HKi;A?&8eD*XTA|bM9#mFCT|>HLxB6QZ@)kT{{m_nwzI)vK<*9a^ z1WpQbL{ufU-?{1>AzL)NkxjU{HKCKHY&2$UJAJ1#@4;=AgRD>TE{I62zJVSCT=C)z zizX7$^#PToG<&ivQf=ZRQ{l9|#wQYS+{U`Ghny<^E)p`&;|zCe^unjRH@EY4f$jCwG?=3RzXHY!5_rF?M<}9m?`sMX z_N0mR+_#N|P!0kKW=na6aP=o{xj$*Zc(PIVE3pkCAej5WO&lFaZlUc&OtX2Fg-uTo8{xUl0rfGh z3Dr;W_og51d>}lK;$&FH;xaeFIC%<#CcSZm*>{eIYnM(IjWedxK*X+@1NTXWP57!Ls$e zV3q}TU-g%SA1OPzGAvYo>Nw8L_KTu#t=d|Bm2t2Js%ZK&l%LzeSYTZh~_$`j>0UC{<$YeRR;>Tv6+_ z$s`%?gvgm*|C29yQgH9Qn* zic*;P0O0W?+#DEn#AJdXSxUl&tY}^R^-Dj`qMbbq3lAB1X6#B!%DTU!X-VjOv@PdGmtRnh}(@*Ou*&Tg z0aA1a&6N`_pH7gd2z$t)|AePd1Qlf~%?Mo0wR{+P!-4ZJd~b!E`ykb}&(HNGkgxyt zQGe+@H2KacJSg6JjL%O9@bDFu7;t}0tXyv)Q8CcCw^<&xH(n1j4+nJdrr1a~2*E@d-?I|d%g~0H;GL{8 zt;J+I=w>c$nv~vdAdr#pq#I8CN%}M}*GGgDjdOcIMgB-H=oOQX9@kW-*3x|z&yKo( z+b~_6Mya%zu0_!0teatIbI#&363J86JGcrI;R*LXjf)VSX?6Ma-}|F(!kd>u>P%8- zmZ5(&t7v8+;x6Ms$wH4ePTT8quyC~TGk_JB?>!L0!4=LMzCKVwCEfS&CO}nV5^yS9 zU0{IZxIFn8GoXPzwHz%fq*XROR!zmS@9V4@Ov7CUDBmp`FdJvL6&I}Okn6Mvg*!^A zUX1YWIT=b3br4M&Wk{Z<3xEsQu~pUks`7@Q29+t2SOJzM%7xKuNsAu_zkHj>Da@wO z?8t1~NEp;?UX1@xlht+cjrDsLLAeag=3*f4M3@~XK|V@xo8gyzh3?SI+;b&ch7+F? za8yrQhuBH{-ZEz3lc4lUjQi5604`3%H~ZI!CG2k`@eEfZ&BBe56%V@oo&ZGxaExg9 zNcKF5o|ecl3vs6cLBB#=@3>aJ-ed4?9KeTfa`cFvRd)Zp$=@wKFG-)~5?va0S$cI8 z#c6mpQ!Wla?F-n`_P%^?w!571x&$t&LP6;J!dRxR$Uq-M*43mhJp!T1P~9F4`i&ZpwU3;Gq(Gv}nbGnj6=u7mr*W>1$ zmfY$6ZE^Gxh$*A~gR2q^0Zz0#B#bI}haePX385VJN*16Q-;b>~bQStjDJ%Z@x)R^; z!AW}l_Yolzl}E4Zerk8WYTcrU%^TzR^9=nvPoxx+x{n-V>BjmNXDBwv@Pv&UW!OH_ zh?36Vqv`Zfp?S(`D{#~OGGz<&O?5gh^ga>tzS8vID=j^Aa1r9KvgP@kAe?kdN@Qxz z7L)VQ-}q~T%rQyvuegK$AnvsZGi8F(wc=+|G(TdVZ0eCc;Bej-z}~xf&TE_a?_>9J zj`?{YWu@?2`p@P=My)ow?fK{3al*dvm-Xz&S^QZBStqISO@eeB9F3Ns9q3^_ z>|@2I^T{+N=D5r(g96!93A!mHF{-o5r5q;VfjO&>cpe1qk|i~;@E02!j;T%w5Mb6> zTMmvSs$>MW+Dqm>A7zg61oF1)rf;;2B4vt_7R8H7pdE4?=I?jxR+Dvz zqSO?~c_L|M>vw1-i+|EA%v@uc;Y>E3-b(XWKpYxH|9YXI4C+IQaqPyNZ<~;?0s~6r z9}^-Mt3(ndKh8Q;8!g1zT9uar3{m2ZQdi%LinZ1_mqU*_;+guD#W7=5NBM=cigAxG zX_%IgC#b8jkhl#my*Juo%YS^7G9vVmDX&txED93|qB`;=ISXX1M<tJ9)i=vWkS94*KYd*qcwbAXaik}$A0n6%s%!{ilFWBd2S+>QrewYxRN zc)yUNjKVS9u7u|iapK%fnpKut?s2B=@Do=7m&+ZsME*)epRse_^ZpLa@T(pXRryyCn3>>n~O#|_tdP`JShBi*`~<>Qnr_KcWM z(#2xOn^&$&gW%@6ozY53L(ozEn#*F2yGH+p`#e9a^pXjdBe8fJ+^jO~_BK3Yq?=3e zRvBPQ>SGtK!oFm>+HCM2sR1~V^5^Jgl>@UzgcyweMSu4=v;N` z>YQ&25mNFWt+2_2Hf?(Re7LPzWA3>>D-=bdah*Qt(Y$odKKCw~**IxsS`m(q&({wy z?+xdM=b=U$+!))>sgisAF4p03yQg&Zu(xvLlrKEiPq-|Dt z0KBa%`0@%#f)NMP-);k`q0RKjHZl_SN}Kbi8QHBgw|z;5b;__!=|3aHzBXf|TLQOE ztyVDuvGG&hgbqEb^M`#5Q^)O2B@nOQx}b6i94V$z2uDwMZ7{HV1GYR|YFE5~O?4*+ za;$yBX8)wEof%fy$}CJzMn&C|frEU?`6vyuF%O!-3|aYB*N}5O$KQ=yF8cx)(AVwl zizq&M5TeS~S%0nMG;#D~HGG%5=d=&x2f6g;6x>qkZ+U=onhPr5K_lWApa>%3+ln09 zY2~9%$$UBo#YK@c1hl>@BE#odeM(-bcIph#PT=!~XTPGg3VR2P6Gcs?W^0rIo@V6} z_x^~QmLM+UbbU0pR!IDrC2Y0}I=^B&V|l`te?v{HCEr%B#jj@Tr%eSEm3F@pO_~;G zC0@cAOed2$_7}da67>9K8(z;~4uQFu$g-&bN|Ub!w4Cm0^>D)2mC6t?Hu873UHlbHhFY|N3q5W0tD{da>bEb)`g4Ib$2GEOe^v$d@sh=8Iy zm4DP7}CJptw12&pFC|~vGXzW+02&qf@oU5-5pYtg?`(zxr%NTS; zOA^CuOaWbQv-nJ2p5n9QS4O`5!FA}PWm>}O8@G1}D$kmgpAqt{er9PLT(o*~{8g5F z4cj!Pyd~&|b;w;+I5MW9zRJcHthr{NLVg#nJQ~J7q`Bbjt=zQ`7W`}vx;!e|(TS1B zQt?s_)J>ed?uYwfkj%rz@wH=W{Lp;8E0@Ji*?^EQRHuv>S~G+3#QNFXef-^_l*OCt z1TB@vxyOF0HDpZraf0MyZlVQ~tly0Dg4p)>JEL!43|DH{(`93`1mst)d~xiCkS88e@yu}(#V3J|I14{>6|wGgXI1jNDT*MJ zZ|L8QC*v*7^%F_;k$I0J3MXF^?0!Yu;U-#Qr`vYC-;Em)t19z4erGXT){C%oO+HKz zvD#mm&J%-8e`*^cQ9&77D$`PB-Usv4HA1SxvUPqnpMi{M32ifN@iLi*B zSgM!Hm59Wc4s3qmSYehXTHw%1O_3GmNM$Pt0A`dq(r=FNl2*k5yKf&q)s3 zPA}JdX{_E1iKn?6F^hLJN`5HoL0S1*V$NCQbXUmmJvea3G*`BXSZC}zpOb9*Bz*A8 zr^i-A3swi6z$e461-zvHd>n-%b1pg^NBDuYp#fZj@ekFL&APh9HMyJ317KNpwnR_{ zF(eDjqQ6cF^QHo0%1381r1xBvpFv#QQXTc}gVWRa25(PkDxIwvlX5x^v{4}T=qAM< zmC!^lX*?`90$g+Jxrg3v{mHBzD>!CQBdd_&x?C#@*{J-D52ozih6!S@jp0P@pLK`S zno7$jDL|sGy)v-rSal+8#~GyQE1K}o8SndjLpex#EO)CW`^6L2-oTnbd2RQfVCyaS zDBCwPLTOwWqf26`+ZSg14&4MyCbS`sFdsyJrI485zRxu{Q5@20E1Xr~F14GGv?_)| zzKN&mllz~@Xy51hh$U85W>EOLj6Mx`mf5!Vy_kXfxB*}aCTC95KI;H4Q^+2C@`f;S2?jT{F<(rc*PnHsF zfohQLG`!n&cx;>1z(Lxxm)H5Ms8Xk_cB9zR^JFv}TaY_Q?}2AE-n|vSiw}++aher; zjkPSu-z7^1W5I+%cemdjc-AdN>l+%pYH75U;5~uENEAId{*EVNIfdzS+7PQU)}oOu z`s1f!qWQw5=ObAcL4N*V)g4gey^I8b>A?=e$s9PRC6^`ODD3e}os@{|L^%4k?a+*8 z)67SXM7%1aFd?o>nkxBx`ZHNtwIB9Ix}H*qoYMEi7;alRo$q`}KBweJJ)iUCt1mqJ zi-|Vt;tMS^^!7laPy0|6zy&`?ysE}-t7-LmkC}1+jO4GO;GC{9QPizEYuE3k{#;0bVA1=_b|`1uKl2x6^=tD{RrK3^42cT&mlNzI(OcEjhLXXu@L-2Bo`h z;tEx4#u)u=C9yqYE9?t$3paxEpL3h|pNP84By%R)@eB&%m<&7J>)j-~$M5*7x>F$P3w7<4fm$j3=l7&+rGGd#C zUr-0_>CCac&U_8h|K}TjZrtzSUF6h{vkc+OV0OM$`KpYP{cBVjj24Un-4GSvG#~EYZewYIDoXKSOWG!*{x&COX+r~B zPsOVyCZMBFdtr55gs$3WE7|eqsn?%# zo~6FugJmo$b4M(`xh6F~T>71wJdHs=-O3j7cj%fAw3W&)XPZyba3~)eR9HdE#H@{> z{2_X^p1Ng;mw}%P^Bz>`yC7q?P(pUdjF%HUObDA3gG+2qfS97D&|~>C`A|m2n{NOx zbi9$1AJ_^D)Mck?{Ha%w*>KTUYnwEIJOoZ2x8r|-XtV{sUoOmLWzl{#o{R`ju3y8< z>OpoeiOpKao7lXjOq9(@yXGeRP=6nSKGAsWn@d;oV_T*9a^a!6riH04PYScanfS$0 z=9$~cI{Mu^h%_lh?il?gX0Di(6QK(3?W@aN(pK3zCWQ}ctf49K>~ALBJnCM4XCT*33RI; z*(hS{b9oga69v6UDfimy;q}~+_nvE$+iPV4EOh~0Lcm)SS6vna#2A6*j6AQt@19rz zm{0gC{Ym8q?10(63W*v{kqnfe*Y?6vt`vWh*^ekr6|IrnQm$(%UgBAFt6F4;W`#PH z@s{B4o31?l+5rPblfxKT`?n5tT91i_G9;#2xnr#Zem>U~7s3@sNlTRjOVGtDeKM!| z9swOfEa}cJPve`Oedjg(qeXBBJ+eH*_ZD^DYvI4FJKkJ7BP6-b3a->RwcGaA$e23) z+~4S4(3OG499dXU=v8>>bZKpZDEo)VxYuyC2~f^TMZMce>uc6~kG&A*Tik*C%+^W8 zWE<4w2@?>x{YB+-cQt|uU;auX5cZ9&+?AJ)th;B#R{6em^im;*e>Y*$*dwCk>zc?n zSAS5JWE?#hEOnKKW;UcT);5HbtWt@->L+m0RXpG1n6;$(rxFe$w?1-IF zB@^U!$p`(aUAq_7s^w&qc+6~s_=B`vpV8r?f$8a*z_}-u_`y7j)4;!afJTRJt|7fD z)Mrjyn+7tpOz}X<_FU2DHHEp~sBy&;XA>Tx=+oLNDlOz`5L=A+PDk7`6`>V5jKNOY z02kxg49bw=j_%njas70SKVaLaWQU%jJ@z)+x>*XA2wC!yH7 z?>v&5#24}zxFKB53Fh{9GiaT&!%PD)K>gFM+ADxW19-XYTuq1oFWSmY#!(I1C~QDi zo2rkc2G_;GvM_QhOPoQUI6y(+5htm-{Dh*TPQ#HP(UJ(!n(FhPA*@bC&5R^64eH|< zwwOA+Z>12Qjn;bn?Ah8sx|m9iB0eduS-nmOgmv3W$cT>yxNPiAw!vU>)<`;Yzcx{B z)BrVuZ@1Y4WZk+wSXO-G{hP35YG4*)RW=4uc^x%9El$(FX##Qd)~?|R&7At{Dt^h9 zFWc0H!&s9YPJBnR>RL5uECGb|lm-i>^0d91o-C4+$rjqsL{bm6i~Qzi=u-Pd$*Y0> zsf7fN96cd5;NUQl3BBFKd}efcvEd{K8{5Y!vg=c&S}rY0>}%p zrR`KN-t&%1yr9}xkzoi1QUwnG?JI8r(bQT@M;feLUTig_<7)9u{L*R^3iirngLqAc zT{H&RgQ$9^CY@vTDtrj1$pXRPhvx{bfsT>a&>syfwo^aTOW#lO``T!ZNicJ5P7jKn zqVfO?D+&vqEDe8VTxRunm<6}G7etyGl`E8AKIY-qssDsg0`megpVHkW(T3g00bAV$hTAlo?J`p3mAXH=DPxXW4^Ij#4?ovEWrk)lgU0G* zb(tf`Qxl7rrr$L@x&QdC5(5E|$1T2WaXU%p7V(ARLKt7;@@Go+fFtX|y|4#&#_fHc zJG=S2X{smsZ*J`7edd}$!xS~}T!fr!Yvu6fJSqEA)oh|RvuXFC0JN4aLe2(A*OuOm+)a8UhofQ2@#5s&dIEE8mzzFWVFDx}wRpoSiL%Tk^_} z?C6OslCywNLw*|m`u}O?%>UWU_Bj5GBDPv5#Td)Xb;~f+7|XQNs9L6UYP+@6E**&( zX%b_tpfrjWt*J7K3N>xTUWSB-wX~LEtU(g76t$#LTkN?fbMIepf4JvAXiEvv1M+#>>tuAhhp zDlr##{za(8Pf_zO7iVrnRt3}Ca9Cv$dBmW_7e|Ec|Bx7noepiHiZUJyPGY*r!v<^# ziVItUu^Poz4Gx-H+veM-Xui1V_T(w{u_bcBKn^(W?{k!)vx8pO_69!udRBSu zgrBiy_Bj%98V}}+HC%d0e`j;o@q86QLv&0D+hY6IBCdu^jv z#SI}>hwHi5eGIQ0G1gtV^C#}JMV5)Wwpy$wj5y<)+FxJFeRuu1LVA&nTFx90Kj@)? zEl)A5J)+SaE=Mu;R)8#5lG+=!Yf=lnV{kfT_oG~d2+fTu&h|+#aUJ45i;kanIx?^B zE-8FC1pDe2gO&O+>QaBh47ahD+Nh8_xvJ00FCR7CjChW8Z)E~qK&3Ld;AWLg22+ag zLT_)%@O(_lmR)Qg%ktIRLf?NR#4h{5HWO*2Fr`AQQr%+C_mf=<?r#XxEkoB9!<%o)rww3|NwePc`vC_vLn%Bon9P|v?ic;j zE@Qj2L15MetWFDEuAA5$y&*X9SCRXbW??z4B zwo}ne#-7!wy$TP~{}y_D-av#fF^4OczGd;bBBLr*BRTxwB!Xdf4HG3dzLAX?3m-zM zjV^pFHOBH{)ZEhuarNHB87_r^*)_)_+RasHXndzOd4mm%54T4rFc!|S%A~2a!jKjx zlch`cY_o<&Lxyk8A9CTlzhV$AIq1<8Y;{?~H1S9MMCruX$~;_dksoIV z$5`FtV~}hHe4Gs1pDg|$!Vk*WFh+?`-FG;FLD#DrL)c;WquAzujtn@gQ{3<r=j6+abF_V! zQCpKE3;z3!7ZCjY)*ar`irzA%c#6hG)LwO=N`EYViDSGILn7QgGsGfM9RyPu@TZET z8mvq)50ayuCy!Z&ap=uVmp!iu*<`8yhTe>mEZB<<{MQAwaNfaU{L^siER#e71T<9q-DCGC88064rX$ z>V>Goj8|#xuE)=WgmyBye_p_sxBhta>tDmweeLdN{U+TS;x-q(`aBakvY=h^;C#9( z^bbaiC1Q8rFi88VipKX|2QU1Yz4#3V%8VS&q1QV zfhLc$ze+|pqe9dm2nju~3iDKKb9CW*UN#E{yRPMpPO1H$nf~OsifwG!90X|(&b~^b zZv>)+-t~Zn8|Ad_G!aNC-N-_sH?6>j^?Dr;T#&|d@GJKy+A8Omqz}-*w>>_Te@e{jO$WBIBOxeL zH`pb9eI0H8o6LV?g|<6S9h!gKzhBvx*<>(Wnm8~An7LvllA$7c4y94=g7-J&_3{?1cIl< zwr5|YM-_W<4er81aUeSwGBU0smq5ZG86;ilR$A$@dbtpzrSgT>=%WmHnB>2{HCZy^22DFJ0IZu*DX zij}XaJ~0mwFXLjX8CiYLYax}h4?+>c)p&Tn;7p!3Rp)KVki|Rr&~ceAz8@%fLO2bS p&~7xg@gxpV-Twdm&pluiAdo3$%FlitJ>DxU#NO7?rphXS_-}a6s)PUl literal 0 HcmV?d00001 From d8aa7868902994fb784cb73818933a9667ea6515 Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Thu, 5 Oct 2023 10:40:10 +0000 Subject: [PATCH 02/15] Witgen: Extract block production from Generator into VmProcessor --- executor/src/witgen/generator.rs | 528 ++------------------------- executor/src/witgen/mod.rs | 1 + executor/src/witgen/vm_processor.rs | 537 ++++++++++++++++++++++++++++ 3 files changed, 561 insertions(+), 505 deletions(-) create mode 100644 executor/src/witgen/vm_processor.rs diff --git a/executor/src/witgen/generator.rs b/executor/src/witgen/generator.rs index 1a33a8d815..5121b88b7f 100644 --- a/executor/src/witgen/generator.rs +++ b/executor/src/witgen/generator.rs @@ -1,65 +1,24 @@ use ast::analyzed::{Identity, IdentityKind, PolyID, PolynomialReference, SelectedExpressions}; -use itertools::Itertools; -use number::{DegreeType, FieldElement}; -use parser_util::lines::indent; -use std::cmp::max; +use number::FieldElement; use std::collections::{HashMap, HashSet}; -use std::time::Instant; - -use crate::witgen::identity_processor::{self, IdentityProcessor}; -use crate::witgen::rows::RowUpdater; use super::affine_expression::AffineExpression; use super::column_map::WitnessColumnMap; use super::identity_processor::Machines; use super::machines::Machine; -use super::query_processor::QueryProcessor; use super::range_constraints::RangeConstraint; use super::machines::FixedLookup; -use super::rows::{transpose_rows, Row, RowFactory, RowPair, UnknownStrategy}; -use super::{EvalError, EvalResult, EvalValue, FixedData, MutableState}; - -/// Phase in which [Generator::compute_row] is called. -#[derive(Debug, PartialEq)] -enum ProcessingPhase { - Initialization, - Regular, -} - -/// A list of identities with a flag whether it is complete. -struct CompletableIdentities<'a, T: FieldElement> { - identities_with_complete: Vec<(&'a Identity, bool)>, -} - -impl<'a, T: FieldElement> CompletableIdentities<'a, T> { - fn new(identities: impl Iterator>) -> Self { - Self { - identities_with_complete: identities.map(|identity| (identity, false)).collect(), - } - } - - /// Yields immutable references to the identity and mutable references to the complete flag. - fn iter_mut(&mut self) -> impl Iterator, &mut bool)> { - self.identities_with_complete - .iter_mut() - .map(|(identity, complete)| (*identity, complete)) - } -} +use super::rows::{transpose_rows, Row, RowFactory}; +use super::vm_processor::VmProcessor; +use super::{EvalResult, FixedData, MutableState}; pub struct Generator<'a, T: FieldElement> { - /// The witness columns belonging to this machine - witnesses: HashSet, fixed_data: &'a FixedData<'a, T>, - /// The subset of identities that contains a reference to the next row - /// (precomputed once for performance reasons) - identities_with_next_ref: Vec<&'a Identity>, - /// The subset of identities that does not contain a reference to the next row - /// (precomputed once for performance reasons) - identities_without_next_ref: Vec<&'a Identity>, + identities: Vec<&'a Identity>, + witnesses: HashSet, + global_range_constraints: WitnessColumnMap>>, data: Vec>, - last_report: DegreeType, - last_report_time: Instant, } impl<'a, T: FieldElement> Machine<'a, T> for Generator<'a, T> { @@ -95,474 +54,33 @@ impl<'a, T: FieldElement> Generator<'a, T> { witnesses: HashSet, global_range_constraints: &WitnessColumnMap>>, ) -> Self { - let row_factory = RowFactory::new(fixed_data, global_range_constraints.clone()); - let default_row = row_factory.fresh_row(); - - let (identities_with_next, identities_without_next): (Vec<_>, Vec<_>) = identities - .iter() - .partition(|identity| identity.contains_next_ref()); - - Generator { - witnesses, + Self { fixed_data, - identities_with_next_ref: identities_with_next, - identities_without_next_ref: identities_without_next, - data: vec![default_row; fixed_data.degree as usize], - last_report: 0, - last_report_time: Instant::now(), + identities: identities.to_vec(), + witnesses, + global_range_constraints: global_range_constraints.clone(), + data: vec![], } } - fn last_row(&self) -> DegreeType { - self.fixed_data.degree - 1 - } - pub fn run(&mut self, mutable_state: &mut MutableState<'a, T, Q>) where Q: FnMut(&str) -> Option + Send + Sync, { - // For identities like `pc' = (1 - first_step') * <...>`, we need to process the last - // row before processing the first row. - self.compute_row( - self.last_row(), - ProcessingPhase::Initialization, - mutable_state, - ); - - // Are we in an infinite loop and can just re-use the old values? - let mut looping_period = None; - let mut loop_detection_log_level = log::Level::Info; - for row_index in 0..self.fixed_data.degree as DegreeType { - self.maybe_log_performance(row_index); - // Check if we are in a loop. - if looping_period.is_none() && row_index % 100 == 0 && row_index > 0 { - looping_period = self.rows_are_repeating(row_index); - if let Some(p) = looping_period { - log::log!( - loop_detection_log_level, - "Found loop with period {p} starting at row {row_index}" - ); - } - } - if let Some(period) = looping_period { - let proposed_row = self.data[row_index as usize - period].clone(); - if !self.try_proposed_row(row_index, proposed_row, mutable_state) { - log::log!( - loop_detection_log_level, - "Looping failed. Trying to generate regularly again. (Use RUST_LOG=debug to see whether this happens more often.)" - ); - looping_period = None; - // For some programs, loop detection will often find loops and then fail. - // In this case, we don't want to spam the user with debug messages. - loop_detection_log_level = log::Level::Debug; - } - } - if looping_period.is_none() { - self.compute_row(row_index, ProcessingPhase::Regular, mutable_state); - }; - } - } - - /// Checks if the last rows are repeating and returns the period. - /// Only checks for periods of 1, 2, 3 and 4. - fn rows_are_repeating(&self, row_index: DegreeType) -> Option { - if row_index < 4 { - return None; - } - - let row = row_index as usize; - (1..=3).find(|&period| { - (1..=period).all(|i| { - self.data[row - i - period] - .values() - .zip(self.data[row - i].values()) - .all(|(a, b)| a.value == b.value) - }) - }) - } - - // Returns a reference to a given row - fn row(&self, row_index: DegreeType) -> &Row<'a, T> { - let row_index = (row_index + self.fixed_data.degree) % self.fixed_data.degree; - &self.data[row_index as usize] - } - - fn compute_row( - &mut self, - row_index: DegreeType, - phase: ProcessingPhase, - mutable_state: &mut MutableState<'a, T, Q>, - ) where - Q: FnMut(&str) -> Option + Send + Sync, - { - log::trace!("Row: {}", row_index); - - let mut identity_processor = IdentityProcessor::new( - self.fixed_data, - &mut mutable_state.fixed_lookup, - mutable_state.machines.iter_mut().into(), - ); - let mut query_processor = mutable_state - .query_callback - .as_mut() - .map(|query| QueryProcessor::new(self.fixed_data, query)); - - log::trace!(" Going over all identities until no more progress is made"); - // First, go over identities that don't reference the next row, - // Second, propagate values to the next row by going over identities that do reference the next row. - let mut identities_without_next_ref = - CompletableIdentities::new(self.identities_without_next_ref.iter().cloned()); - let mut identities_with_next_ref = - CompletableIdentities::new(self.identities_with_next_ref.iter().cloned()); - self.loop_until_no_progress( - row_index, - &mut identities_without_next_ref, - &mut identity_processor, - &mut query_processor, - ) - .and_then(|_| { - self.loop_until_no_progress( - row_index, - &mut identities_with_next_ref, - &mut identity_processor, - &mut query_processor, - ) - }) - .map_err(|e| self.report_failure_and_panic_unsatisfiable(row_index, e)) - .unwrap(); - - // Check that the computed row is "final" by asserting that all unknown values can - // be set to 0. - // This check is skipped in the initialization phase (run on the last row), - // because its only purpose is to transfer values to the first row, - // not to finalize the last row. - if phase == ProcessingPhase::Regular { - log::trace!( - " Checking that remaining identities hold when unknown values are set to 0" - ); - self.process_identities( - row_index, - &mut identities_without_next_ref, - UnknownStrategy::Zero, - &mut identity_processor, - ) - .and_then(|_| { - self.process_identities( - row_index, - &mut identities_with_next_ref, - UnknownStrategy::Zero, - &mut identity_processor, - ) - }) - .map_err(|e| self.report_failure_and_panic_underconstrained(row_index, e)) - .unwrap(); - } - - log::trace!( - "{}", - self.row(row_index) - .render(&format!("===== Row {}", row_index), true, &self.witnesses) - ); - } - - /// Loops over all identities and queries, until no further progress is made. - /// @returns the "incomplete" identities, i.e. identities that contain unknown values. - fn loop_until_no_progress( - &mut self, - row_index: DegreeType, - identities: &mut CompletableIdentities<'a, T>, - identity_processor: &mut IdentityProcessor<'a, '_, T>, - query_processor: &mut Option>, - ) -> Result<(), Vec>> - where - Q: FnMut(&str) -> Option + Send + Sync, - { - loop { - let mut progress = self.process_identities( - row_index, - identities, - UnknownStrategy::Unknown, - identity_processor, - )?; - if let Some(query_processor) = query_processor.as_mut() { - let row_pair = RowPair::new( - self.row(row_index), - self.row(row_index + 1), - row_index, - self.fixed_data, - UnknownStrategy::Unknown, - ); - let updates = query_processor.process_queries_on_current_row(&row_pair); - progress |= self.apply_updates(row_index, &updates, || "query".to_string()); - } - - if !progress { - break; - } - } - Ok(()) - } - - /// Loops over all identities once and updates the current row and next row. - /// Arguments: - /// * `identities`: Identities to process. Completed identities are removed from the list. - /// * `unknown_strategy`: How to process unknown variables. Either use zero or keep it symbolic. - /// Returns: - /// * `Ok(true)`: If progress was made. - /// * `Ok(false)`: If no progress was made. - /// * `Err(errors)`: If an error occurred. - fn process_identities( - &mut self, - row_index: DegreeType, - identities: &mut CompletableIdentities<'a, T>, - unknown_strategy: UnknownStrategy, - identity_processor: &mut IdentityProcessor<'a, '_, T>, - ) -> Result>> { - let mut progress = false; - let mut errors = vec![]; - - for (identity, is_complete) in identities.iter_mut() { - if *is_complete { - continue; - } - - let is_machine_call = matches!( - identity.kind, - IdentityKind::Plookup | IdentityKind::Permutation - ); - if is_machine_call && unknown_strategy == UnknownStrategy::Zero { - // The fact that we got to the point where we assume 0 for unknown cells, but this identity - // is still not complete, means that either the inputs or the machine is under-constrained. - errors.push(format!("{identity}:\n{}", - indent("This machine call could not be completed. Either some inputs are missing or the machine is under-constrained.", " ")).into()); - continue; - } - - let row_pair = RowPair::new( - self.row(row_index), - self.row(row_index + 1), - row_index, - self.fixed_data, - unknown_strategy, - ); - let result: EvalResult<'a, T> = identity_processor - .process_identity(identity, &row_pair) - .map_err(|err| { - format!("{identity}:\n{}", indent(&format!("{err}"), " ")).into() - }); - - match result { - Ok(eval_value) => { - if unknown_strategy == UnknownStrategy::Zero { - assert!(eval_value.constraints.is_empty()) - } else { - *is_complete = eval_value.is_complete(); - progress |= - self.apply_updates(row_index, &eval_value, || format!("{identity}")); - } - } - Err(e) => { - errors.push(e); - } - }; - } - - if errors.is_empty() { - Ok(progress) - } else { - Err(errors) - } - } - - fn apply_updates( - &mut self, - row_index: DegreeType, - updates: &EvalValue<&PolynomialReference, T>, - source_name: impl Fn() -> String, - ) -> bool { - let (before, after) = if row_index == self.last_row() { - // Last row is current, first row is next - let (after, before) = self.data.split_at_mut(row_index as usize); - (before, after) - } else { - self.data.split_at_mut(row_index as usize + 1) - }; - let current = before.last_mut().unwrap(); - let next = after.first_mut().unwrap(); - let mut row_updater = RowUpdater::new(current, next, row_index); - row_updater.apply_updates(updates, source_name) - } - - fn report_failure_and_panic_unsatisfiable( - &self, - row_index: DegreeType, - failures: Vec>, - ) -> ! { - log::error!( - "\nError: Row {} failed. Set RUST_LOG=debug for more information.\n", - row_index - ); - log::debug!("Some identities where not satisfiable after the following values were uniquely determined (known nonzero first, then zero, unknown omitted):"); - log::debug!( - "{}", - self.row(row_index) - .render("Current Row", false, &self.witnesses) - ); - log::debug!( - "{}", - self.row(row_index + 1) - .render("Next Row", false, &self.witnesses) - ); - log::debug!("Set RUST_LOG=trace to understand why these values were chosen."); - log::debug!( - "Assuming these values are correct, the following identities fail:\n{}\n", - failures - .iter() - .map(|r| indent(&r.to_string(), " ")) - .join("\n") - ); - panic!("Witness generation failed."); - } - - fn report_failure_and_panic_underconstrained( - &self, - row_index: DegreeType, - failures: Vec>, - ) -> ! { - log::error!( - "\nError: Row {} failed. Set RUST_LOG=debug for more information.\n", - row_index - ); - - log::debug!("Some columns could not be determined, but setting them to zero does not satisfy the constraints. This typically means that the system is underconstrained!"); - log::debug!( - "{}", - self.row(row_index) - .render("Current Row", true, &self.witnesses) - ); - log::debug!( - "{}", - self.row(row_index) - .render("Next Row", true, &self.witnesses) - ); - log::debug!("\nSet RUST_LOG=trace to understand why these values were (not) chosen."); - log::debug!( - "Assuming zero for unknown values, the following identities fail:\n{}\n", - failures - .iter() - .map(|r| indent(&r.to_string(), " ")) - .join("\n") - ); - panic!("Witness generation failed."); - } + // For now, run the VM Processor on an empty block of the size of the degree + // In the future, we'll instantate a processor for each block and then stitch them together here. + let row_factory = RowFactory::new(self.fixed_data, self.global_range_constraints.clone()); + let default_row = row_factory.fresh_row(); + let data = vec![default_row; self.fixed_data.degree as usize]; - /// Verifies the proposed values for the next row. - /// TODO this is bad for machines because we might introduce rows in the machine that are then - /// not used. - fn try_proposed_row( - &mut self, - row_index: DegreeType, - proposed_row: Row<'a, T>, - mutable_state: &mut MutableState<'a, T, Q>, - ) -> bool - where - Q: FnMut(&str) -> Option + Send + Sync, - { - let mut identity_processor = IdentityProcessor::new( + let mut processor = VmProcessor::new( self.fixed_data, - &mut mutable_state.fixed_lookup, - mutable_state.machines.iter_mut().into(), + &self.identities, + self.witnesses.clone(), + data, ); - let constraints_valid = - self.check_row_pair(row_index, &proposed_row, false, &mut identity_processor) - && self.check_row_pair(row_index, &proposed_row, true, &mut identity_processor); + processor.run(mutable_state); - if constraints_valid { - self.data[row_index as usize] = proposed_row; - } else { - // Note that we never update the next row if proposing a row succeeds (the happy path). - // If it doesn't, we re-run compute_next_row on the previous row in order to - // correctly forward-propagate values via next references. - self.compute_row(row_index - 1, ProcessingPhase::Regular, mutable_state); - } - constraints_valid - } - - fn check_row_pair( - &self, - row_index: DegreeType, - proposed_row: &Row<'a, T>, - previous: bool, - identity_processor: &mut IdentityProcessor<'a, '_, T>, - ) -> bool { - let row_pair = match previous { - // Check whether identities with a reference to the next row are satisfied - // when applied to the previous row and the proposed row. - true => RowPair::new( - self.row(row_index - 1), - proposed_row, - row_index - 1, - self.fixed_data, - UnknownStrategy::Zero, - ), - // Check whether identities without a reference to the next row are satisfied - // when applied to the proposed row. - // Note that we also provide the next row here, but it is not used. - false => RowPair::new( - proposed_row, - self.row(row_index + 1), - row_index, - self.fixed_data, - UnknownStrategy::Zero, - ), - }; - - // Check identities depending on whether or not they have a reference to the next row: - // - Those that do should be checked on the previous and proposed row - // - All others should be checked on the proposed row - let identities = match previous { - true => &self.identities_with_next_ref, - false => &self.identities_without_next_ref, - }; - - for identity in identities.iter() { - if identity_processor - .process_identity(identity, &row_pair) - .is_err() - { - log::debug!("Previous {:?}", self.row(row_index - 1)); - log::debug!("Proposed {:?}", proposed_row); - log::debug!("Failed on identity: {}", identity); - - return false; - } - } - true - } - - fn maybe_log_performance(&mut self, row_index: DegreeType) { - if row_index >= self.last_report + 1000 { - let duration = self.last_report_time.elapsed(); - self.last_report_time = Instant::now(); - - let identity_statistics = identity_processor::get_and_reset_solving_statistics(); - let identities_per_sec = - ((identity_statistics.values().map(|s| s.success).sum::() as u128 * 1000) - / duration.as_micros()) as u64; - let identities_count = max(identity_statistics.len() as u64, 1); - let progress_percentage = identity_statistics - .values() - .map(|s| s.success * 100 / s.invocations) - .sum::() - / identities_count; - - log::info!( - "{row_index} of {} rows ({}%) - {} rows/s, {identities_per_sec}k identities/s, {progress_percentage}% progress", - self.fixed_data.degree, - row_index * 100 / self.fixed_data.degree, - 1_000_000_000 / duration.as_micros() - ); - self.last_report = row_index; - } + self.data = processor.finish(); } } diff --git a/executor/src/witgen/mod.rs b/executor/src/witgen/mod.rs index f9884e7ef5..c17724ce04 100644 --- a/executor/src/witgen/mod.rs +++ b/executor/src/witgen/mod.rs @@ -34,6 +34,7 @@ mod sequence_iterator; pub mod symbolic_evaluator; mod symbolic_witness_evaluator; mod util; +mod vm_processor; /// Everything [Generator] needs to mutate in order to compute a new row. pub struct MutableState<'a, T: FieldElement, Q: FnMut(&str) -> Option + Send + Sync> { diff --git a/executor/src/witgen/vm_processor.rs b/executor/src/witgen/vm_processor.rs new file mode 100644 index 0000000000..39d493f8a4 --- /dev/null +++ b/executor/src/witgen/vm_processor.rs @@ -0,0 +1,537 @@ +use ast::analyzed::{Identity, IdentityKind, PolyID, PolynomialReference}; +use itertools::Itertools; +use number::{DegreeType, FieldElement}; +use parser_util::lines::indent; +use std::cmp::max; +use std::collections::HashSet; +use std::time::Instant; + +use crate::witgen::identity_processor::{self, IdentityProcessor}; +use crate::witgen::rows::RowUpdater; + +use super::query_processor::QueryProcessor; + +use super::rows::{Row, RowPair, UnknownStrategy}; +use super::{EvalError, EvalResult, EvalValue, FixedData, MutableState}; + +/// Phase in which [VmProcessor::compute_row] is called. +#[derive(Debug, PartialEq)] +enum ProcessingPhase { + Initialization, + Regular, +} + +/// A list of identities with a flag whether it is complete. +struct CompletableIdentities<'a, T: FieldElement> { + identities_with_complete: Vec<(&'a Identity, bool)>, +} + +impl<'a, T: FieldElement> CompletableIdentities<'a, T> { + fn new(identities: impl Iterator>) -> Self { + Self { + identities_with_complete: identities.map(|identity| (identity, false)).collect(), + } + } + + /// Yields immutable references to the identity and mutable references to the complete flag. + fn iter_mut(&mut self) -> impl Iterator, &mut bool)> { + self.identities_with_complete + .iter_mut() + .map(|(identity, complete)| (*identity, complete)) + } +} + +pub struct VmProcessor<'a, T: FieldElement> { + /// The witness columns belonging to this machine + witnesses: HashSet, + fixed_data: &'a FixedData<'a, T>, + /// The subset of identities that contains a reference to the next row + /// (precomputed once for performance reasons) + identities_with_next_ref: Vec<&'a Identity>, + /// The subset of identities that does not contain a reference to the next row + /// (precomputed once for performance reasons) + identities_without_next_ref: Vec<&'a Identity>, + data: Vec>, + last_report: DegreeType, + last_report_time: Instant, +} + +impl<'a, T: FieldElement> VmProcessor<'a, T> { + pub fn new( + fixed_data: &'a FixedData<'a, T>, + identities: &[&'a Identity], + witnesses: HashSet, + data: Vec>, + ) -> Self { + let (identities_with_next, identities_without_next): (Vec<_>, Vec<_>) = identities + .iter() + .partition(|identity| identity.contains_next_ref()); + + VmProcessor { + witnesses, + fixed_data, + identities_with_next_ref: identities_with_next, + identities_without_next_ref: identities_without_next, + data, + last_report: 0, + last_report_time: Instant::now(), + } + } + + pub fn finish(self) -> Vec> { + self.data + } + + fn last_row(&self) -> DegreeType { + self.fixed_data.degree - 1 + } + + pub fn run(&mut self, mutable_state: &mut MutableState<'a, T, Q>) + where + Q: FnMut(&str) -> Option + Send + Sync, + { + // For identities like `pc' = (1 - first_step') * <...>`, we need to process the last + // row before processing the first row. + self.compute_row( + self.last_row(), + ProcessingPhase::Initialization, + mutable_state, + ); + + // Are we in an infinite loop and can just re-use the old values? + let mut looping_period = None; + let mut loop_detection_log_level = log::Level::Info; + for row_index in 0..self.fixed_data.degree as DegreeType { + self.maybe_log_performance(row_index); + // Check if we are in a loop. + if looping_period.is_none() && row_index % 100 == 0 && row_index > 0 { + looping_period = self.rows_are_repeating(row_index); + if let Some(p) = looping_period { + log::log!( + loop_detection_log_level, + "Found loop with period {p} starting at row {row_index}" + ); + } + } + if let Some(period) = looping_period { + let proposed_row = self.data[row_index as usize - period].clone(); + if !self.try_proposed_row(row_index, proposed_row, mutable_state) { + log::log!( + loop_detection_log_level, + "Looping failed. Trying to generate regularly again. (Use RUST_LOG=debug to see whether this happens more often.)" + ); + looping_period = None; + // For some programs, loop detection will often find loops and then fail. + // In this case, we don't want to spam the user with debug messages. + loop_detection_log_level = log::Level::Debug; + } + } + if looping_period.is_none() { + self.compute_row(row_index, ProcessingPhase::Regular, mutable_state); + }; + } + } + + /// Checks if the last rows are repeating and returns the period. + /// Only checks for periods of 1, 2, 3 and 4. + fn rows_are_repeating(&self, row_index: DegreeType) -> Option { + if row_index < 4 { + return None; + } + + let row = row_index as usize; + (1..=3).find(|&period| { + (1..=period).all(|i| { + self.data[row - i - period] + .values() + .zip(self.data[row - i].values()) + .all(|(a, b)| a.value == b.value) + }) + }) + } + + // Returns a reference to a given row + fn row(&self, row_index: DegreeType) -> &Row<'a, T> { + let row_index = (row_index + self.fixed_data.degree) % self.fixed_data.degree; + &self.data[row_index as usize] + } + + fn compute_row( + &mut self, + row_index: DegreeType, + phase: ProcessingPhase, + mutable_state: &mut MutableState<'a, T, Q>, + ) where + Q: FnMut(&str) -> Option + Send + Sync, + { + log::trace!("Row: {}", row_index); + + let mut identity_processor = IdentityProcessor::new( + self.fixed_data, + &mut mutable_state.fixed_lookup, + mutable_state.machines.iter_mut().into(), + ); + let mut query_processor = mutable_state + .query_callback + .as_mut() + .map(|query| QueryProcessor::new(self.fixed_data, query)); + + log::trace!(" Going over all identities until no more progress is made"); + // First, go over identities that don't reference the next row, + // Second, propagate values to the next row by going over identities that do reference the next row. + let mut identities_without_next_ref = + CompletableIdentities::new(self.identities_without_next_ref.iter().cloned()); + let mut identities_with_next_ref = + CompletableIdentities::new(self.identities_with_next_ref.iter().cloned()); + self.loop_until_no_progress( + row_index, + &mut identities_without_next_ref, + &mut identity_processor, + &mut query_processor, + ) + .and_then(|_| { + self.loop_until_no_progress( + row_index, + &mut identities_with_next_ref, + &mut identity_processor, + &mut query_processor, + ) + }) + .map_err(|e| self.report_failure_and_panic_unsatisfiable(row_index, e)) + .unwrap(); + + // Check that the computed row is "final" by asserting that all unknown values can + // be set to 0. + // This check is skipped in the initialization phase (run on the last row), + // because its only purpose is to transfer values to the first row, + // not to finalize the last row. + if phase == ProcessingPhase::Regular { + log::trace!( + " Checking that remaining identities hold when unknown values are set to 0" + ); + self.process_identities( + row_index, + &mut identities_without_next_ref, + UnknownStrategy::Zero, + &mut identity_processor, + ) + .and_then(|_| { + self.process_identities( + row_index, + &mut identities_with_next_ref, + UnknownStrategy::Zero, + &mut identity_processor, + ) + }) + .map_err(|e| self.report_failure_and_panic_underconstrained(row_index, e)) + .unwrap(); + } + + log::trace!( + "{}", + self.row(row_index) + .render(&format!("===== Row {}", row_index), true, &self.witnesses) + ); + } + + /// Loops over all identities and queries, until no further progress is made. + /// @returns the "incomplete" identities, i.e. identities that contain unknown values. + fn loop_until_no_progress( + &mut self, + row_index: DegreeType, + identities: &mut CompletableIdentities<'a, T>, + identity_processor: &mut IdentityProcessor<'a, '_, T>, + query_processor: &mut Option>, + ) -> Result<(), Vec>> + where + Q: FnMut(&str) -> Option + Send + Sync, + { + loop { + let mut progress = self.process_identities( + row_index, + identities, + UnknownStrategy::Unknown, + identity_processor, + )?; + if let Some(query_processor) = query_processor.as_mut() { + let row_pair = RowPair::new( + self.row(row_index), + self.row(row_index + 1), + row_index, + self.fixed_data, + UnknownStrategy::Unknown, + ); + let updates = query_processor.process_queries_on_current_row(&row_pair); + progress |= self.apply_updates(row_index, &updates, || "query".to_string()); + } + + if !progress { + break; + } + } + Ok(()) + } + + /// Loops over all identities once and updates the current row and next row. + /// Arguments: + /// * `identities`: Identities to process. Completed identities are removed from the list. + /// * `unknown_strategy`: How to process unknown variables. Either use zero or keep it symbolic. + /// Returns: + /// * `Ok(true)`: If progress was made. + /// * `Ok(false)`: If no progress was made. + /// * `Err(errors)`: If an error occurred. + fn process_identities( + &mut self, + row_index: DegreeType, + identities: &mut CompletableIdentities<'a, T>, + unknown_strategy: UnknownStrategy, + identity_processor: &mut IdentityProcessor<'a, '_, T>, + ) -> Result>> { + let mut progress = false; + let mut errors = vec![]; + + for (identity, is_complete) in identities.iter_mut() { + if *is_complete { + continue; + } + + let is_machine_call = matches!( + identity.kind, + IdentityKind::Plookup | IdentityKind::Permutation + ); + if is_machine_call && unknown_strategy == UnknownStrategy::Zero { + // The fact that we got to the point where we assume 0 for unknown cells, but this identity + // is still not complete, means that either the inputs or the machine is under-constrained. + errors.push(format!("{identity}:\n{}", + indent("This machine call could not be completed. Either some inputs are missing or the machine is under-constrained.", " ")).into()); + continue; + } + + let row_pair = RowPair::new( + self.row(row_index), + self.row(row_index + 1), + row_index, + self.fixed_data, + unknown_strategy, + ); + let result: EvalResult<'a, T> = identity_processor + .process_identity(identity, &row_pair) + .map_err(|err| { + format!("{identity}:\n{}", indent(&format!("{err}"), " ")).into() + }); + + match result { + Ok(eval_value) => { + if unknown_strategy == UnknownStrategy::Zero { + assert!(eval_value.constraints.is_empty()) + } else { + *is_complete = eval_value.is_complete(); + progress |= + self.apply_updates(row_index, &eval_value, || format!("{identity}")); + } + } + Err(e) => { + errors.push(e); + } + }; + } + + if errors.is_empty() { + Ok(progress) + } else { + Err(errors) + } + } + + fn apply_updates( + &mut self, + row_index: DegreeType, + updates: &EvalValue<&PolynomialReference, T>, + source_name: impl Fn() -> String, + ) -> bool { + let (before, after) = if row_index == self.last_row() { + // Last row is current, first row is next + let (after, before) = self.data.split_at_mut(row_index as usize); + (before, after) + } else { + self.data.split_at_mut(row_index as usize + 1) + }; + let current = before.last_mut().unwrap(); + let next = after.first_mut().unwrap(); + let mut row_updater = RowUpdater::new(current, next, row_index); + row_updater.apply_updates(updates, source_name) + } + + fn report_failure_and_panic_unsatisfiable( + &self, + row_index: DegreeType, + failures: Vec>, + ) -> ! { + log::error!( + "\nError: Row {} failed. Set RUST_LOG=debug for more information.\n", + row_index + ); + log::debug!("Some identities where not satisfiable after the following values were uniquely determined (known nonzero first, then zero, unknown omitted):"); + log::debug!( + "{}", + self.row(row_index) + .render("Current Row", false, &self.witnesses) + ); + log::debug!( + "{}", + self.row(row_index + 1) + .render("Next Row", false, &self.witnesses) + ); + log::debug!("Set RUST_LOG=trace to understand why these values were chosen."); + log::debug!( + "Assuming these values are correct, the following identities fail:\n{}\n", + failures + .iter() + .map(|r| indent(&r.to_string(), " ")) + .join("\n") + ); + panic!("Witness generation failed."); + } + + fn report_failure_and_panic_underconstrained( + &self, + row_index: DegreeType, + failures: Vec>, + ) -> ! { + log::error!( + "\nError: Row {} failed. Set RUST_LOG=debug for more information.\n", + row_index + ); + + log::debug!("Some columns could not be determined, but setting them to zero does not satisfy the constraints. This typically means that the system is underconstrained!"); + log::debug!( + "{}", + self.row(row_index) + .render("Current Row", true, &self.witnesses) + ); + log::debug!( + "{}", + self.row(row_index) + .render("Next Row", true, &self.witnesses) + ); + log::debug!("\nSet RUST_LOG=trace to understand why these values were (not) chosen."); + log::debug!( + "Assuming zero for unknown values, the following identities fail:\n{}\n", + failures + .iter() + .map(|r| indent(&r.to_string(), " ")) + .join("\n") + ); + panic!("Witness generation failed."); + } + + /// Verifies the proposed values for the next row. + /// TODO this is bad for machines because we might introduce rows in the machine that are then + /// not used. + fn try_proposed_row( + &mut self, + row_index: DegreeType, + proposed_row: Row<'a, T>, + mutable_state: &mut MutableState<'a, T, Q>, + ) -> bool + where + Q: FnMut(&str) -> Option + Send + Sync, + { + let mut identity_processor = IdentityProcessor::new( + self.fixed_data, + &mut mutable_state.fixed_lookup, + mutable_state.machines.iter_mut().into(), + ); + let constraints_valid = + self.check_row_pair(row_index, &proposed_row, false, &mut identity_processor) + && self.check_row_pair(row_index, &proposed_row, true, &mut identity_processor); + + if constraints_valid { + self.data[row_index as usize] = proposed_row; + } else { + // Note that we never update the next row if proposing a row succeeds (the happy path). + // If it doesn't, we re-run compute_next_row on the previous row in order to + // correctly forward-propagate values via next references. + self.compute_row(row_index - 1, ProcessingPhase::Regular, mutable_state); + } + constraints_valid + } + + fn check_row_pair( + &self, + row_index: DegreeType, + proposed_row: &Row<'a, T>, + previous: bool, + identity_processor: &mut IdentityProcessor<'a, '_, T>, + ) -> bool { + let row_pair = match previous { + // Check whether identities with a reference to the next row are satisfied + // when applied to the previous row and the proposed row. + true => RowPair::new( + self.row(row_index - 1), + proposed_row, + row_index - 1, + self.fixed_data, + UnknownStrategy::Zero, + ), + // Check whether identities without a reference to the next row are satisfied + // when applied to the proposed row. + // Note that we also provide the next row here, but it is not used. + false => RowPair::new( + proposed_row, + self.row(row_index + 1), + row_index, + self.fixed_data, + UnknownStrategy::Zero, + ), + }; + + // Check identities depending on whether or not they have a reference to the next row: + // - Those that do should be checked on the previous and proposed row + // - All others should be checked on the proposed row + let identities = match previous { + true => &self.identities_with_next_ref, + false => &self.identities_without_next_ref, + }; + + for identity in identities.iter() { + if identity_processor + .process_identity(identity, &row_pair) + .is_err() + { + log::debug!("Previous {:?}", self.row(row_index - 1)); + log::debug!("Proposed {:?}", proposed_row); + log::debug!("Failed on identity: {}", identity); + + return false; + } + } + true + } + + fn maybe_log_performance(&mut self, row_index: DegreeType) { + if row_index >= self.last_report + 1000 { + let duration = self.last_report_time.elapsed(); + self.last_report_time = Instant::now(); + + let identity_statistics = identity_processor::get_and_reset_solving_statistics(); + let identities_per_sec = + ((identity_statistics.values().map(|s| s.success).sum::() as u128 * 1000) + / duration.as_micros()) as u64; + let identities_count = max(identity_statistics.len() as u64, 1); + let progress_percentage = identity_statistics + .values() + .map(|s| s.success * 100 / s.invocations) + .sum::() + / identities_count; + + log::info!( + "{row_index} of {} rows ({}%) - {} rows/s, {identities_per_sec}k identities/s, {progress_percentage}% progress", + self.fixed_data.degree, + row_index * 100 / self.fixed_data.degree, + 1_000_000_000 / duration.as_micros() + ); + self.last_report = row_index; + } + } +} From 263c03d77d77fa72986f2a7cba159f4df95d0367 Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Thu, 5 Oct 2023 14:47:07 +0000 Subject: [PATCH 03/15] WrapGL machine: return both high and low values --- compiler/tests/powdr_std.rs | 4 +- std/mod.asm | 2 +- std/split/mod.asm | 1 + std/{wrap/wrap_gl.asm => split/split_gl.asm} | 14 +++--- std/wrap/mod.asm | 1 - test_data/std/split_gl_test.asm | 52 ++++++++++++++++++++ test_data/std/wrap_gl_test.asm | 46 ----------------- 7 files changed, 64 insertions(+), 56 deletions(-) create mode 100644 std/split/mod.asm rename std/{wrap/wrap_gl.asm => split/split_gl.asm} (84%) delete mode 100644 std/wrap/mod.asm create mode 100644 test_data/std/split_gl_test.asm delete mode 100644 test_data/std/wrap_gl_test.asm diff --git a/compiler/tests/powdr_std.rs b/compiler/tests/powdr_std.rs index b0b0fca10f..43516b43f7 100644 --- a/compiler/tests/powdr_std.rs +++ b/compiler/tests/powdr_std.rs @@ -62,8 +62,8 @@ fn poseidon_gl_test() { } #[test] -fn wrap_gl_test() { - let f = "wrap_gl_test.asm"; +fn split_gl_test() { + let f = "split_gl_test.asm"; verify_asm::(f, Default::default()); gen_estark_proof(f, Default::default()); } diff --git a/std/mod.asm b/std/mod.asm index 4403092bcf..000d5c115e 100644 --- a/std/mod.asm +++ b/std/mod.asm @@ -1,4 +1,4 @@ mod binary; mod hash; mod shift; -mod wrap; \ No newline at end of file +mod split; \ No newline at end of file diff --git a/std/split/mod.asm b/std/split/mod.asm new file mode 100644 index 0000000000..5e04b8d4c4 --- /dev/null +++ b/std/split/mod.asm @@ -0,0 +1 @@ +mod split_gl; \ No newline at end of file diff --git a/std/wrap/wrap_gl.asm b/std/split/split_gl.asm similarity index 84% rename from std/wrap/wrap_gl.asm rename to std/split/split_gl.asm index d02342d5b6..5a51b1243d 100644 --- a/std/wrap/wrap_gl.asm +++ b/std/split/split_gl.asm @@ -1,7 +1,7 @@ -// Wraps an arbitrary field element into a u32, on the Goldilocks field. -machine WrapGL(RESET, operation_id) { +// Splits an arbitrary field element into two u32s, on the Goldilocks field. +machine SplitGL(RESET, operation_id) { - operation wrap<0> in_acc -> output; + operation split<0> in_acc -> output_low, output_high; // Latch and operation ID col fixed RESET(i) { i % 8 == 7 }; @@ -22,9 +22,11 @@ machine WrapGL(RESET, operation_id) { // 2. Build the output, packing the least significant 4 byte into // a field element - col witness output; - col fixed FACTOR_OUTPUT = [0x100, 0x10000, 0x1000000, 0, 0, 0, 0, 1]*; - output' = (1 - RESET) * output + bytes * FACTOR_OUTPUT; + col witness output_low, output_high; + col fixed FACTOR_OUTPUT_LOW = [0x100, 0x10000, 0x1000000, 0, 0, 0, 0, 1]*; + col fixed FACTOR_OUTPUT_HIGH = [0, 0, 0, 1, 0x100, 0x10000, 0x1000000, 0]*; + output_low' = (1 - RESET) * output_low + bytes * FACTOR_OUTPUT_LOW; + output_high' = (1 - RESET) * output_high + bytes * FACTOR_OUTPUT_HIGH; // 3. Check that the byte decomposition does not overflow // diff --git a/std/wrap/mod.asm b/std/wrap/mod.asm deleted file mode 100644 index e2cc6b5771..0000000000 --- a/std/wrap/mod.asm +++ /dev/null @@ -1 +0,0 @@ -mod wrap_gl; \ No newline at end of file diff --git a/test_data/std/split_gl_test.asm b/test_data/std/split_gl_test.asm new file mode 100644 index 0000000000..239051b553 --- /dev/null +++ b/test_data/std/split_gl_test.asm @@ -0,0 +1,52 @@ +use std::split::split_gl::SplitGL; + + +machine Main { + reg pc[@pc]; + reg X0[<=]; + reg X1[<=]; + reg X2[<=]; + reg low; + reg high; + + degree 65536; + + SplitGL split_machine; + + instr split X0 -> X1, X2 = split_machine.split + + instr assert_eq X0, X1 { + X0 = X1 + } + + instr loop { pc' = pc } + + function main { + + // Min value + // Note that this has two byte decompositions, 0x and p = 0xffffffff00000001. + // The second would lead to a different split value, but should be ruled + // out by the overflow check. + low, high <== split(0); + assert_eq low, 0; + assert_eq high, 0; + + // Max value + // On Goldilocks, this is 0xffffffff00000000. + low, high <== split(-1); + assert_eq low, 0; + assert_eq high, 0xffffffff; + + // Max low value + low, high <== split(0xfffffffeffffffff); + assert_eq low, 0xffffffff; + assert_eq high, 0xfffffffe; + + // Some other value + low, high <== split(0xabcdef0123456789); + assert_eq low, 0x23456789; + assert_eq high, 0xabcdef01; + + return; + } +} \ No newline at end of file diff --git a/test_data/std/wrap_gl_test.asm b/test_data/std/wrap_gl_test.asm deleted file mode 100644 index 67cfc1a120..0000000000 --- a/test_data/std/wrap_gl_test.asm +++ /dev/null @@ -1,46 +0,0 @@ -use std::wrap::wrap_gl::WrapGL; - - -machine Main { - reg pc[@pc]; - reg X0[<=]; - reg X1[<=]; - reg A; - - degree 65536; - - WrapGL wrap_machine; - - instr wrap X0 -> X1 = wrap_machine.wrap - - instr assert_eq X0, X1 { - X0 = X1 - } - - instr loop { pc' = pc } - - function main { - - // Min value - // Note that this has two byte decompositions, 0x and p = 0xffffffff00000001. - // The second would lead to a different wrapped value, but should be ruled - // out by the overflow check. - A <== wrap(0); - assert_eq A, 0; - - // Max value - // On Goldilocks, this is 0xffffffff00000000, which wraps to 0. - A <== wrap(-1); - assert_eq A, 0; - - // Max wrapped value - A <== wrap(0xfffffffeffffffff); - assert_eq A, 0xffffffff; - - // Some other value - A <== wrap(0xabcdef0123456789); - assert_eq A, 0x23456789; - - return; - } -} \ No newline at end of file From 1f62c35491d613f2838c7c7d52324763a404f249 Mon Sep 17 00:00:00 2001 From: schaeff Date: Thu, 5 Oct 2023 11:08:29 +0200 Subject: [PATCH 04/15] detect circular dependency --- importer/src/path_canonicalizer.rs | 112 ++++++++++++++---- importer/test_data/cycle.asm | 11 ++ importer/test_data/import_after_usage.asm | 9 ++ .../test_data/import_after_usage.expected.asm | 7 ++ 4 files changed, 113 insertions(+), 26 deletions(-) create mode 100644 importer/test_data/cycle.asm create mode 100644 importer/test_data/import_after_usage.asm create mode 100644 importer/test_data/import_after_usage.expected.asm diff --git a/importer/src/path_canonicalizer.rs b/importer/src/path_canonicalizer.rs index 28764ae008..5755895cc9 100644 --- a/importer/src/path_canonicalizer.rs +++ b/importer/src/path_canonicalizer.rs @@ -9,7 +9,7 @@ use std::{ use ast::parsed::{ asm::{ ASMModule, ASMProgram, AbsoluteSymbolPath, Import, Machine, MachineStatement, Module, - ModuleRef, ModuleStatement, SymbolDefinition, SymbolPath, SymbolValue, SymbolValueRef, + ModuleRef, ModuleStatement, SymbolDefinition, SymbolValue, SymbolValueRef, }, folder::Folder, }; @@ -81,7 +81,7 @@ impl<'a, T> Folder for Canonicalizer<'a> { if let MachineStatement::Submachine(_, path, _) = s { *path = self .paths - .get(&(self.path.clone(), std::mem::take(path))) + .get(&self.path.clone().join(std::mem::take(path))) .cloned() .unwrap() .into(); @@ -92,8 +92,8 @@ impl<'a, T> Folder for Canonicalizer<'a> { } } -/// Answers the question: When at absolute path `p`, refering to relative path `r`, what is the absolute path to the canonical symbol imported? -pub type PathMap = BTreeMap<(AbsoluteSymbolPath, SymbolPath), AbsoluteSymbolPath>; +/// For each imported absolute path, the absolute path to the canonical symbol +pub type PathMap = BTreeMap; /// The state of the checking process. We visit the module tree collecting each relative path and pointing it to the absolute path it resolves to in the state. #[derive(PartialEq, Debug)] @@ -104,6 +104,33 @@ pub struct State<'a, T> { pub paths: PathMap, } +#[derive(Default)] +struct PathDependencyChain { + paths: Vec, +} + +impl PathDependencyChain { + fn push(&mut self, path: AbsoluteSymbolPath) -> Result<(), String> { + match self.paths.iter().position(|p| p == &path) { + Some(index) => { + self.paths.push(path); + Err(format!( + "Cycle detected in `use` statements: {}", + self.paths[index..] + .iter() + .map(|p| format!("`{p}`")) + .collect::>() + .join(" -> ") + )) + } + None => { + self.paths.push(path); + Ok(()) + } + } + } +} + /// Checks a relative path in the context of an absolute path, if successful returning an updated state containing the absolute path /// /// # Panics @@ -114,27 +141,36 @@ pub struct State<'a, T> { /// /// This function will return an error if the relative path does not resolve to anything fn check_path( - // the location at which the import is made - location: AbsoluteSymbolPath, - // the imported path, relative to the location - imported: SymbolPath, + // the path to check + path: AbsoluteSymbolPath, // the current state state: State<'_, T>, -) -> Result<(State<'_, T>, AbsoluteSymbolPath, SymbolValueRef<'_, T>), String> { + // the locations visited so far + mut chain: PathDependencyChain, +) -> Result< + ( + State<'_, T>, + AbsoluteSymbolPath, + SymbolValueRef<'_, T>, + PathDependencyChain, + ), + String, +> { let root = state.root.clone(); + + chain.push(path.clone())?; + // walk down the tree of modules - location - .clone() - .join(imported.clone()) - .parts + path.parts .iter() .try_fold( ( state, AbsoluteSymbolPath::default(), SymbolValueRef::Module(ModuleRef::Local(root)), + chain, ), - |(state, mut location, value), member| { + |(state, mut location, value, chain), member| { match value { // machines do not expose symbols SymbolValueRef::Machine(_) => { @@ -151,11 +187,16 @@ fn check_path( match symbol { SymbolValue::Import(p) => { // if we found an import, check it and continue from there - check_path(location, p.path.clone(), state) + check_path(location.join(p.path.clone()), state, chain) } symbol => { // if we found any other symbol, continue from there - Ok((state, location.join(member.clone()), symbol.as_ref())) + Ok(( + state, + location.join(member.clone()), + symbol.as_ref(), + chain, + )) } } }), @@ -165,16 +206,18 @@ fn check_path( location.pop_back().unwrap(); // redirect to `p` - check_path(location, p.path.clone().join(member.clone()), state) + check_path( + location.join(p.path.clone().join(member.clone())), + state, + chain, + ) } } }, ) - .map(|(mut state, absolute_path, symbol)| { - state - .paths - .insert((location, imported), absolute_path.clone()); - (state, absolute_path, symbol) + .map(|(mut state, canonical_path, symbol, chain)| { + state.paths.insert(path, canonical_path.clone()); + (state, canonical_path, symbol, chain) }) } @@ -191,7 +234,11 @@ fn check_import( // the current state state: State<'_, T>, ) -> Result, String> { - let (state, _, _) = check_path(location, imported.path, state)?; + let (state, _, _, _) = check_path( + location.join(imported.path), + state, + PathDependencyChain::default(), + )?; Ok(state) } @@ -270,9 +317,12 @@ fn check_machine<'a, T: Clone>( m.statements .iter() .try_fold(state, |state, statement| match statement { - MachineStatement::Submachine(_, path, _) => { - check_path(module_location.clone(), path.clone(), state).map(|(state, _, _)| state) - } + MachineStatement::Submachine(_, path, _) => check_path( + module_location.clone().join(path.clone()), + state, + PathDependencyChain::default(), + ) + .map(|(state, _, _, _)| state), _ => Ok(state), }) } @@ -399,4 +449,14 @@ mod tests { fn usage_chain() { expect("usage_chain", Ok(())) } + + #[test] + fn cycle() { + expect("cycle", Err("Cycle detected in `use` statements: `module::Machine` -> `other_module::submodule::MyMachine` -> `Machine` -> `module::Machine`")) + } + + #[test] + fn import_after_usage() { + expect("import_after_usage", Ok(())) + } } diff --git a/importer/test_data/cycle.asm b/importer/test_data/cycle.asm new file mode 100644 index 0000000000..c3993ff6ca --- /dev/null +++ b/importer/test_data/cycle.asm @@ -0,0 +1,11 @@ +use module::Machine; + +mod module { + use super::other_module::submodule::MyMachine as Machine; +} + +mod other_module { + mod submodule { + use super::super::Machine as MyMachine; + } +} \ No newline at end of file diff --git a/importer/test_data/import_after_usage.asm b/importer/test_data/import_after_usage.asm new file mode 100644 index 0000000000..0b9b86fe8e --- /dev/null +++ b/importer/test_data/import_after_usage.asm @@ -0,0 +1,9 @@ +machine Foo { +} + +mod module { + machine Bar { + Foo foo; + } + use super::Foo; +} \ No newline at end of file diff --git a/importer/test_data/import_after_usage.expected.asm b/importer/test_data/import_after_usage.expected.asm new file mode 100644 index 0000000000..16be99e57b --- /dev/null +++ b/importer/test_data/import_after_usage.expected.asm @@ -0,0 +1,7 @@ +machine Foo { +} +mod module { + machine Bar { + Foo foo; + } +} From 9906a5b937f4eb3a723bd837e0dfd982fc3a8997 Mon Sep 17 00:00:00 2001 From: Leo Alt Date: Mon, 9 Oct 2023 13:05:51 +0200 Subject: [PATCH 05/15] fix extra parameter in starky --- backend/src/pilstark/estark.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/backend/src/pilstark/estark.rs b/backend/src/pilstark/estark.rs index 6fa88785f5..f423b9b74a 100644 --- a/backend/src/pilstark/estark.rs +++ b/backend/src/pilstark/estark.rs @@ -113,6 +113,7 @@ impl BackendImpl for EStark { &setup.program, &pil, &self.params, + &"".to_string(), ) .unwrap(); From 58cce78784da60b677960e11b28f029ff9b84a28 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 9 Oct 2023 12:48:12 +0200 Subject: [PATCH 06/15] Change expression visitor to be more flexible. --- ast/src/analyzed/visitor.rs | 4 ++-- ast/src/asm_analysis/mod.rs | 2 +- ast/src/parsed/visitor.rs | 42 ++++++++++++++++++------------------- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/ast/src/analyzed/visitor.rs b/ast/src/analyzed/visitor.rs index d397b814a3..9d7d2094c8 100644 --- a/ast/src/analyzed/visitor.rs +++ b/ast/src/analyzed/visitor.rs @@ -2,7 +2,7 @@ use crate::parsed::visitor::VisitOrder; use super::*; -impl ExpressionVisitable for Analyzed { +impl ExpressionVisitable> for Analyzed { fn visit_expressions_mut(&mut self, f: &mut F, o: VisitOrder) -> ControlFlow where F: FnMut(&mut parsed::Expression) -> ControlFlow, @@ -50,7 +50,7 @@ impl ExpressionVisitable for Analyzed { } } -impl ExpressionVisitable for Identity { +impl ExpressionVisitable> for Identity { fn visit_expressions_mut(&mut self, f: &mut F, o: VisitOrder) -> ControlFlow where F: FnMut(&mut parsed::Expression) -> ControlFlow, diff --git a/ast/src/asm_analysis/mod.rs b/ast/src/asm_analysis/mod.rs index 19f638ed07..1e55ad13a2 100644 --- a/ast/src/asm_analysis/mod.rs +++ b/ast/src/asm_analysis/mod.rs @@ -540,7 +540,7 @@ pub enum FunctionStatement { Return(Return), } -impl ExpressionVisitable> for FunctionStatement { +impl ExpressionVisitable>> for FunctionStatement { fn visit_expressions_mut(&mut self, f: &mut F, o: VisitOrder) -> std::ops::ControlFlow where F: FnMut(&mut Expression>) -> std::ops::ControlFlow, diff --git a/ast/src/parsed/visitor.rs b/ast/src/parsed/visitor.rs index 06616f6efa..626e4187e9 100644 --- a/ast/src/parsed/visitor.rs +++ b/ast/src/parsed/visitor.rs @@ -14,12 +14,12 @@ pub enum VisitOrder { /// A trait to be implemented by an AST node. /// The idea is that it calls a callback function on each of the sub-nodes /// that are expressions. -pub trait ExpressionVisitable { +pub trait ExpressionVisitable { /// Traverses the AST and calls `f` on each Expression in pre-order, /// potentially break early and return a value. fn pre_visit_expressions_return_mut(&mut self, f: &mut F) -> ControlFlow where - F: FnMut(&mut Expression) -> ControlFlow, + F: FnMut(&mut Expr) -> ControlFlow, { self.visit_expressions_mut(f, VisitOrder::Pre) } @@ -27,7 +27,7 @@ pub trait ExpressionVisitable { /// Traverses the AST and calls `f` on each Expression in pre-order. fn pre_visit_expressions_mut(&mut self, f: &mut F) where - F: FnMut(&mut Expression), + F: FnMut(&mut Expr), { self.pre_visit_expressions_return_mut(&mut move |e| { f(e); @@ -39,7 +39,7 @@ pub trait ExpressionVisitable { /// potentially break early and return a value. fn pre_visit_expressions_return(&self, f: &mut F) -> ControlFlow where - F: FnMut(&Expression) -> ControlFlow, + F: FnMut(&Expr) -> ControlFlow, { self.visit_expressions(f, VisitOrder::Pre) } @@ -47,7 +47,7 @@ pub trait ExpressionVisitable { /// Traverses the AST and calls `f` on each Expression in pre-order. fn pre_visit_expressions(&self, f: &mut F) where - F: FnMut(&Expression), + F: FnMut(&Expr), { self.pre_visit_expressions_return(&mut move |e| { f(e); @@ -59,7 +59,7 @@ pub trait ExpressionVisitable { /// potentially break early and return a value. fn post_visit_expressions_return_mut(&mut self, f: &mut F) -> ControlFlow where - F: FnMut(&mut Expression) -> ControlFlow, + F: FnMut(&mut Expr) -> ControlFlow, { self.visit_expressions_mut(f, VisitOrder::Post) } @@ -67,7 +67,7 @@ pub trait ExpressionVisitable { /// Traverses the AST and calls `f` on each Expression in post-order. fn post_visit_expressions_mut(&mut self, f: &mut F) where - F: FnMut(&mut Expression), + F: FnMut(&mut Expr), { self.post_visit_expressions_return_mut(&mut move |e| { f(e); @@ -79,7 +79,7 @@ pub trait ExpressionVisitable { /// potentially break early and return a value. fn post_visit_expressions_return(&self, f: &mut F) -> ControlFlow where - F: FnMut(&Expression) -> ControlFlow, + F: FnMut(&Expr) -> ControlFlow, { self.visit_expressions(f, VisitOrder::Post) } @@ -87,7 +87,7 @@ pub trait ExpressionVisitable { /// Traverses the AST and calls `f` on each Expression in post-order. fn post_visit_expressions(&self, f: &mut F) where - F: FnMut(&Expression), + F: FnMut(&Expr), { self.post_visit_expressions_return(&mut move |e| { f(e); @@ -97,14 +97,14 @@ pub trait ExpressionVisitable { fn visit_expressions(&self, f: &mut F, order: VisitOrder) -> ControlFlow where - F: FnMut(&Expression) -> ControlFlow; + F: FnMut(&Expr) -> ControlFlow; fn visit_expressions_mut(&mut self, f: &mut F, order: VisitOrder) -> ControlFlow where - F: FnMut(&mut Expression) -> ControlFlow; + F: FnMut(&mut Expr) -> ControlFlow; } -impl ExpressionVisitable for Expression { +impl ExpressionVisitable> for Expression { fn visit_expressions_mut(&mut self, f: &mut F, o: VisitOrder) -> ControlFlow where F: FnMut(&mut Expression) -> ControlFlow, @@ -182,7 +182,7 @@ impl ExpressionVisitable for Expression { } } -impl ExpressionVisitable> for PilStatement { +impl ExpressionVisitable>> for PilStatement { fn visit_expressions_mut(&mut self, f: &mut F, o: VisitOrder) -> ControlFlow where F: FnMut(&mut Expression>) -> ControlFlow, @@ -256,7 +256,7 @@ impl ExpressionVisitable> for PilStatement ExpressionVisitable for SelectedExpressions { +impl ExpressionVisitable> for SelectedExpressions { fn visit_expressions_mut(&mut self, f: &mut F, o: VisitOrder) -> ControlFlow where F: FnMut(&mut Expression) -> ControlFlow, @@ -280,7 +280,7 @@ impl ExpressionVisitable for SelectedExpressions { } } -impl ExpressionVisitable> for FunctionDefinition { +impl ExpressionVisitable> for FunctionDefinition { fn visit_expressions_mut(&mut self, f: &mut F, o: VisitOrder) -> ControlFlow where F: FnMut(&mut Expression) -> ControlFlow, @@ -308,7 +308,7 @@ impl ExpressionVisitable> for FunctionDefini } } -impl ExpressionVisitable> for ArrayExpression { +impl ExpressionVisitable> for ArrayExpression { fn visit_expressions_mut(&mut self, f: &mut F, o: VisitOrder) -> ControlFlow where F: FnMut(&mut Expression) -> ControlFlow, @@ -342,7 +342,7 @@ impl ExpressionVisitable> for ArrayExpressio } } -impl ExpressionVisitable for LambdaExpression { +impl ExpressionVisitable> for LambdaExpression { fn visit_expressions_mut(&mut self, f: &mut F, o: VisitOrder) -> ControlFlow where F: FnMut(&mut Expression) -> ControlFlow, @@ -358,7 +358,7 @@ impl ExpressionVisitable for LambdaExpression { } } -impl ExpressionVisitable for ArrayLiteral { +impl ExpressionVisitable> for ArrayLiteral { fn visit_expressions_mut(&mut self, f: &mut F, o: VisitOrder) -> ControlFlow where F: FnMut(&mut Expression) -> ControlFlow, @@ -378,7 +378,7 @@ impl ExpressionVisitable for ArrayLiteral { } } -impl ExpressionVisitable for FunctionCall { +impl ExpressionVisitable> for FunctionCall { fn visit_expressions_mut(&mut self, f: &mut F, o: VisitOrder) -> ControlFlow where F: FnMut(&mut Expression) -> ControlFlow, @@ -398,7 +398,7 @@ impl ExpressionVisitable for FunctionCall { } } -impl ExpressionVisitable for MatchArm { +impl ExpressionVisitable> for MatchArm { fn visit_expressions_mut(&mut self, f: &mut F, o: VisitOrder) -> ControlFlow where F: FnMut(&mut Expression) -> ControlFlow, @@ -416,7 +416,7 @@ impl ExpressionVisitable for MatchArm { } } -impl ExpressionVisitable for MatchPattern { +impl ExpressionVisitable> for MatchPattern { fn visit_expressions_mut(&mut self, f: &mut F, o: VisitOrder) -> ControlFlow where F: FnMut(&mut Expression) -> ControlFlow, From a9585d2adde2e9c7499ffac6df9677c1f9af7875 Mon Sep 17 00:00:00 2001 From: chriseth Date: Sat, 30 Sep 2023 00:11:53 +0200 Subject: [PATCH 07/15] Extract evaluator and processor. --- executor/src/constant_evaluator/mod.rs | 67 ++------ pil_analyzer/src/evaluator.rs | 94 +++++++++++ pil_analyzer/src/lib.rs | 1 + pil_analyzer/src/pil_analyzer.rs | 210 +++++++++++-------------- 4 files changed, 201 insertions(+), 171 deletions(-) create mode 100644 pil_analyzer/src/evaluator.rs diff --git a/executor/src/constant_evaluator/mod.rs b/executor/src/constant_evaluator/mod.rs index ee3700aebe..bfeab5e137 100644 --- a/executor/src/constant_evaluator/mod.rs +++ b/executor/src/constant_evaluator/mod.rs @@ -1,10 +1,9 @@ use std::collections::HashMap; -use ast::analyzed::{Analyzed, Expression, FunctionValueDefinition, Reference}; -use ast::parsed::{FunctionCall, MatchArm, MatchPattern}; -use ast::{evaluate_binary_operation, evaluate_unary_operation}; +use ast::analyzed::{Analyzed, FunctionValueDefinition}; use itertools::Itertools; use number::{DegreeType, FieldElement}; +use pil_analyzer::evaluator::Evaluator; use rayon::prelude::{IntoParallelIterator, ParallelIterator}; /// Generates the constant polynomial values for all constant polynomials @@ -42,18 +41,21 @@ fn generate_values( .into_par_iter() .map(|i| { Evaluator { - analyzed, + constants: &analyzed.constants, + definitions: &analyzed.definitions, variables: &[i.into()], - other_constants, + function_cache: other_constants, } .evaluate(body) + .unwrap() }) .collect(), FunctionValueDefinition::Array(values) => { let evaluator = Evaluator { - analyzed, + constants: &analyzed.constants, + definitions: &analyzed.definitions, variables: &[], - other_constants, + function_cache: other_constants, }; let values: Vec<_> = values .iter() @@ -61,7 +63,7 @@ fn generate_values( let items = elements .pattern() .iter() - .map(|v| evaluator.evaluate(v)) + .map(|v| evaluator.evaluate(v).unwrap()) .collect::>(); items @@ -81,55 +83,6 @@ fn generate_values( } } -struct Evaluator<'a, T> { - analyzed: &'a Analyzed, - other_constants: &'a HashMap<&'a str, Vec>, - variables: &'a [T], -} - -impl<'a, T: FieldElement> Evaluator<'a, T> { - fn evaluate(&self, expr: &Expression) -> T { - match expr { - Expression::Constant(name) => self.analyzed.constants[name], - Expression::Reference(Reference::LocalVar(i, _name)) => self.variables[*i as usize], - Expression::Reference(Reference::Poly(_)) => todo!(), - Expression::PublicReference(_) => todo!(), - Expression::Number(n) => *n, - Expression::String(_) => panic!(), - Expression::Tuple(_) => panic!(), - Expression::ArrayLiteral(_) => panic!(), - Expression::BinaryOperation(left, op, right) => { - evaluate_binary_operation(self.evaluate(left), *op, self.evaluate(right)) - } - Expression::UnaryOperation(op, expr) => { - evaluate_unary_operation(*op, self.evaluate(expr)) - } - Expression::LambdaExpression(_) => panic!(), - Expression::FunctionCall(FunctionCall { id, arguments }) => { - let arg_values = arguments - .iter() - .map(|a| self.evaluate(a)) - .collect::>(); - assert!(arg_values.len() == 1); - let values = &self.other_constants[id.as_str()]; - values[arg_values[0].to_degree() as usize % values.len()] - } - Expression::MatchExpression(scrutinee, arms) => { - let v = self.evaluate(scrutinee); - arms.iter() - .find_map(|MatchArm { pattern, value }| match pattern { - MatchPattern::Pattern(p) => { - (self.evaluate(p) == v).then(|| self.evaluate(value)) - } - MatchPattern::CatchAll => Some(self.evaluate(value)), - }) - .expect("No arm matched the value {v}") - } - Expression::FreeInput(_) => panic!(), - } - } -} - #[cfg(test)] mod test { use number::GoldilocksField; diff --git a/pil_analyzer/src/evaluator.rs b/pil_analyzer/src/evaluator.rs new file mode 100644 index 0000000000..357315f1aa --- /dev/null +++ b/pil_analyzer/src/evaluator.rs @@ -0,0 +1,94 @@ +use std::collections::HashMap; + +use ast::{ + analyzed::{Analyzed, Expression, FunctionValueDefinition, Reference, Symbol}, + evaluate_binary_operation, evaluate_unary_operation, + parsed::{FunctionCall, MatchArm, MatchPattern}, +}; +use number::FieldElement; + +/// Evaluates an expression to a single value. +pub fn evaluate_expression( + analyzed: &Analyzed, + expression: &Expression, +) -> Result { + Evaluator { + constants: &analyzed.constants, + definitions: &analyzed.definitions, + function_cache: &Default::default(), + variables: &[], + } + .evaluate(expression) +} + +pub struct Evaluator<'a, T> { + pub constants: &'a HashMap, + pub definitions: &'a HashMap>)>, + /// Contains full value tables of functions (columns) we already evaluated. + pub function_cache: &'a HashMap<&'a str, Vec>, + pub variables: &'a [T], +} + +impl<'a, T: FieldElement> Evaluator<'a, T> { + pub fn evaluate(&self, expr: &Expression) -> Result { + match expr { + Expression::Constant(name) => Ok(self.constants[name]), + Expression::Reference(Reference::LocalVar(i, _name)) => Ok(self.variables[*i as usize]), + Expression::Reference(Reference::Poly(poly)) => { + if !poly.next && poly.index.is_none() { + let name = poly.name.to_owned(); + if let Some(value) = self.constants.get(&name) { + Ok(*value) + } else { + let (_, value) = &self.definitions[&name]; + match value { + Some(FunctionValueDefinition::Expression(value)) => { + self.evaluate(value) + } + _ => Err("Cannot evaluate function values".to_string()), + } + } + } else { + Err("Cannot evaluate arrays or next references.".to_string()) + } + } + Expression::PublicReference(r) => Err(format!("Cannot evaluate public reference: {r}")), + Expression::Number(n) => Ok(*n), + Expression::String(_) => Err("Cannot evaluate string literal.".to_string()), + Expression::Tuple(_) => Err("Cannot evaluate tuple.".to_string()), + Expression::ArrayLiteral(_) => Err("Cannot evaluate array literal.".to_string()), + Expression::BinaryOperation(left, op, right) => Ok(evaluate_binary_operation( + self.evaluate(left)?, + *op, + self.evaluate(right)?, + )), + Expression::UnaryOperation(op, expr) => { + Ok(evaluate_unary_operation(*op, self.evaluate(expr)?)) + } + Expression::LambdaExpression(_) => { + Err("Cannot evaluate lambda expression.".to_string()) + } + Expression::FunctionCall(FunctionCall { id, arguments }) => { + let arg_values = arguments + .iter() + .map(|a| self.evaluate(a)) + .collect::, _>>()?; + assert!(arg_values.len() == 1); + let values = &self.function_cache[id.as_str()]; + Ok(values[arg_values[0].to_degree() as usize % values.len()]) + } + Expression::MatchExpression(scrutinee, arms) => { + let v = self.evaluate(scrutinee); + arms.iter() + .find_map(|MatchArm { pattern, value }| match pattern { + MatchPattern::Pattern(p) => { + (self.evaluate(p) == v).then(|| self.evaluate(value)) + } + MatchPattern::CatchAll => Some(self.evaluate(value)), + }) + .expect("No arm matched the value {v}") + } + Expression::FreeInput(_) => Err("Cannot evaluate free input.".to_string()), + } + } +} diff --git a/pil_analyzer/src/lib.rs b/pil_analyzer/src/lib.rs index 92314ef71a..5793fb6ad0 100644 --- a/pil_analyzer/src/lib.rs +++ b/pil_analyzer/src/lib.rs @@ -1,5 +1,6 @@ #![deny(clippy::print_stdout)] +pub mod evaluator; pub mod pil_analyzer; use std::path::Path; diff --git a/pil_analyzer/src/pil_analyzer.rs b/pil_analyzer/src/pil_analyzer.rs index 4ba23a852d..6763ea2eff 100644 --- a/pil_analyzer/src/pil_analyzer.rs +++ b/pil_analyzer/src/pil_analyzer.rs @@ -6,8 +6,8 @@ use analysis::MacroExpander; use ast::parsed::visitor::ExpressionVisitable; use ast::parsed::{ - self, ArrayExpression, ArrayLiteral, BinaryOperator, FunctionDefinition, LambdaExpression, - MatchArm, MatchPattern, PilStatement, PolynomialName, UnaryOperator, + self, ArrayExpression, ArrayLiteral, FunctionDefinition, LambdaExpression, MatchArm, + MatchPattern, PilStatement, PolynomialName, }; use number::{DegreeType, FieldElement}; @@ -17,6 +17,8 @@ use ast::analyzed::{ StatementIdentifier, Symbol, SymbolKind, }; +use crate::evaluator::Evaluator; + pub fn process_pil_file(path: &Path) -> Analyzed { let mut analyzer = PILAnalyzer::new(); analyzer.process_file(path); @@ -46,7 +48,6 @@ struct PILAnalyzer { current_file: PathBuf, symbol_counters: BTreeMap, identity_counter: HashMap, - local_variables: HashMap, macro_expander: MacroExpander, } @@ -196,7 +197,7 @@ impl PILAnalyzer { ); } PilStatement::ConstantDefinition(_start, name, value) => { - self.handle_constant_definition(name, self.evaluate_expression(&value).unwrap()) + self.handle_constant_definition(name, self.evaluate_expression(value).unwrap()) } PilStatement::LetStatement(start, name, value) => { self.handle_generic_definition(start, name, value) @@ -255,7 +256,7 @@ impl PILAnalyzer { ); } _ => { - if let Some(constant) = self.evaluate_expression(&value) { + if let Ok(constant) = self.evaluate_expression(value.clone()) { // Value evaluates to a constant number => treat it as a constant self.handle_constant_definition(name.to_string(), constant); } else { @@ -288,25 +289,25 @@ impl PILAnalyzer { PilStatement::PlookupIdentity(start, key, haystack) => ( start, IdentityKind::Plookup, - self.process_selected_expression(key), - self.process_selected_expression(haystack), + ExpressionProcessor::new(self).process_selected_expression(key), + ExpressionProcessor::new(self).process_selected_expression(haystack), ), PilStatement::PermutationIdentity(start, left, right) => ( start, IdentityKind::Permutation, - self.process_selected_expression(left), - self.process_selected_expression(right), + ExpressionProcessor::new(self).process_selected_expression(left), + ExpressionProcessor::new(self).process_selected_expression(right), ), PilStatement::ConnectIdentity(start, left, right) => ( start, IdentityKind::Connect, SelectedExpressions { selector: None, - expressions: self.process_expressions(left), + expressions: ExpressionProcessor::new(self).process_expressions(left), }, SelectedExpressions { selector: None, - expressions: self.process_expressions(right), + expressions: ExpressionProcessor::new(self).process_expressions(right), }, ), // TODO at some point, these should all be caught by the type checker. @@ -335,7 +336,7 @@ impl PILAnalyzer { fn handle_namespace(&mut self, name: String, degree: ::ast::parsed::Expression) { // TODO: the polynomial degree should be handled without going through a field element. This requires having types in Expression - self.polynomial_degree = self.evaluate_expression(°ree).unwrap().to_degree(); + self.polynomial_degree = self.evaluate_expression(degree).unwrap().to_degree(); self.namespace = name; } @@ -366,7 +367,7 @@ impl PILAnalyzer { ) -> u64 { let have_array_size = array_size.is_some(); let length = array_size - .map(|l| self.evaluate_expression(&l).unwrap()) + .map(|l| self.evaluate_expression(l).unwrap()) .map(|l| l.to_degree()); if length.is_some() { assert!(value.is_none()); @@ -397,16 +398,21 @@ impl PILAnalyzer { FunctionDefinition::Mapping(params, expr) => { assert!(!have_array_size); assert!(symbol_kind == SymbolKind::Poly(PolynomialType::Constant)); - FunctionValueDefinition::Mapping(self.process_function(¶ms, expr)) + FunctionValueDefinition::Mapping( + ExpressionProcessor::new(self).process_function(¶ms, expr), + ) } FunctionDefinition::Query(params, expr) => { assert!(!have_array_size); assert_eq!(symbol_kind, SymbolKind::Poly(PolynomialType::Committed)); - FunctionValueDefinition::Query(self.process_function(¶ms, expr)) + FunctionValueDefinition::Query( + ExpressionProcessor::new(self).process_function(¶ms, expr), + ) } FunctionDefinition::Array(value) => { let size = value.solve(self.polynomial_degree); - let expression = self.process_array_expression(value, size); + let expression = + ExpressionProcessor::new(self).process_array_expression(value, size); assert_eq!( expression.iter().map(|e| e.size()).sum::(), self.polynomial_degree @@ -424,33 +430,6 @@ impl PILAnalyzer { id } - fn process_function( - &mut self, - params: &[String], - expression: ::ast::parsed::Expression, - ) -> Expression { - let previous_local_vars = std::mem::take(&mut self.local_variables); - - assert!(self.local_variables.is_empty()); - self.local_variables = params - .iter() - .enumerate() - .map(|(i, p)| (p.clone(), i as u64)) - .collect(); - // Re-add the outer local variables if we do not overwrite them - // and increase their index by the number of parameters. - // TODO re-evaluate if this mechanism makes sense as soon as we properly - // support nested functions and closures. - for (name, index) in &previous_local_vars { - self.local_variables - .entry(name.clone()) - .or_insert(index + params.len() as u64); - } - let processed_value = self.process_expression(expression); - self.local_variables = previous_local_vars; - processed_value - } - fn handle_public_declaration( &mut self, source: SourceRef, @@ -465,8 +444,9 @@ impl PILAnalyzer { id, source, name: name.to_string(), - polynomial: self.process_namespaced_polynomial_reference(poly), - index: self.evaluate_expression(&index).unwrap().to_degree(), + polynomial: ExpressionProcessor::new(self) + .process_namespaced_polynomial_reference(poly), + index: self.evaluate_expression(index).unwrap().to_degree(), }, ); self.source_order @@ -485,7 +465,7 @@ impl PILAnalyzer { id } - fn namespaced(&self, name: &str) -> String { + pub fn namespaced(&self, name: &str) -> String { self.namespaced_ref(&None, name) } @@ -499,7 +479,39 @@ impl PILAnalyzer { } } - fn process_selected_expression( + fn evaluate_expression(&self, expr: ::ast::parsed::Expression) -> Result { + Evaluator { + constants: &self.constants, + definitions: &self.definitions, + function_cache: &Default::default(), + variables: &[], + } + .evaluate(&self.process_expression(expr)) + } + + fn process_expression(&self, expr: ::ast::parsed::Expression) -> Expression { + ExpressionProcessor::new(self).process_expression(expr) + } +} + +/// The ExpressionProcessor turns parsed expressions into analyzed expressions. +/// Its main job is to resolve references: +/// It turns simple references into fully namespaced references and resolves local function variables. +/// It also evaluates expressions that are required to be compile-time constant. +struct ExpressionProcessor<'a, T> { + analyzer: &'a PILAnalyzer, + local_variables: HashMap, +} + +impl<'a, T: FieldElement> ExpressionProcessor<'a, T> { + fn new(analyzer: &'a PILAnalyzer) -> Self { + Self { + analyzer, + local_variables: Default::default(), + } + } + + pub fn process_selected_expression( &mut self, expr: ::ast::parsed::SelectedExpressions, ) -> SelectedExpressions { @@ -509,7 +521,7 @@ impl PILAnalyzer { } } - fn process_array_expression( + pub fn process_array_expression( &mut self, array_expression: ::ast::parsed::ArrayExpression, size: DegreeType, @@ -538,7 +550,7 @@ impl PILAnalyzer { } } - fn process_expressions( + pub fn process_expressions( &mut self, exprs: Vec<::ast::parsed::Expression>, ) -> Vec> { @@ -548,7 +560,7 @@ impl PILAnalyzer { .collect() } - fn process_expression(&mut self, expr: ::ast::parsed::Expression) -> Expression { + pub fn process_expression(&mut self, expr: ::ast::parsed::Expression) -> Expression { use ast::parsed::Expression as PExpression; match expr { PExpression::Constant(name) => Expression::Constant(name), @@ -586,7 +598,7 @@ impl PILAnalyzer { Expression::UnaryOperation(op, Box::new(self.process_expression(*value))) } PExpression::FunctionCall(c) => Expression::FunctionCall(parsed::FunctionCall { - id: self.namespaced(&c.id), + id: self.analyzer.namespaced(&c.id), arguments: self.process_expressions(c.arguments), }), PExpression::MatchExpression(scrutinee, arms) => Expression::MatchExpression( @@ -607,16 +619,43 @@ impl PILAnalyzer { } } - fn process_namespaced_polynomial_reference( - &self, + fn process_function( + &mut self, + params: &[String], + expression: ::ast::parsed::Expression, + ) -> Expression { + let previous_local_vars = std::mem::take(&mut self.local_variables); + + assert!(self.local_variables.is_empty()); + self.local_variables = params + .iter() + .enumerate() + .map(|(i, p)| (p.clone(), i as u64)) + .collect(); + // Re-add the outer local variables if we do not overwrite them + // and increase their index by the number of parameters. + // TODO re-evaluate if this mechanism makes sense as soon as we properly + // support nested functions and closures. + for (name, index) in &previous_local_vars { + self.local_variables + .entry(name.clone()) + .or_insert(index + params.len() as u64); + } + let processed_value = self.process_expression(expression); + self.local_variables = previous_local_vars; + processed_value + } + + pub fn process_namespaced_polynomial_reference( + &mut self, poly: ::ast::parsed::NamespacedPolynomialReference, ) -> PolynomialReference { let index = poly .index() .as_ref() - .map(|i| self.evaluate_expression(i).unwrap()) + .map(|i| self.analyzer.evaluate_expression(*i.clone()).unwrap()) .map(|i| i.to_degree()); - let name = self.namespaced_ref(poly.namespace(), poly.name()); + let name = self.analyzer.namespaced_ref(poly.namespace(), poly.name()); PolynomialReference { name, poly_id: None, @@ -625,8 +664,8 @@ impl PILAnalyzer { } } - fn process_shifted_polynomial_reference( - &self, + pub fn process_shifted_polynomial_reference( + &mut self, poly: ::ast::parsed::ShiftedPolynomialReference, ) -> PolynomialReference { PolynomialReference { @@ -634,63 +673,6 @@ impl PILAnalyzer { ..self.process_namespaced_polynomial_reference(poly.into_namespaced()) } } - - fn evaluate_expression(&self, expr: &::ast::parsed::Expression) -> Option { - use ast::parsed::Expression::*; - match expr { - Constant(name) => Some( - *self - .constants - .get(name) - .unwrap_or_else(|| panic!("Constant {name} not found.")), - ), - Reference(name) => { - // TODO this whole mechanism should be replaced by a generic "reference" - // type plus operators. - if !name.shift() && name.namespace().is_none() { - // See if it might be a constant - self.constants.get(&name.name().to_owned()).cloned() - } else { - None - } - } - PublicReference(_) => None, - Number(n) => Some(*n), - String(_) => None, - Tuple(_) => None, - ArrayLiteral(_) => None, - LambdaExpression(_) => None, - BinaryOperation(left, op, right) => self.evaluate_binary_operation(left, *op, right), - UnaryOperation(op, value) => self.evaluate_unary_operation(*op, value), - FunctionCall(_) => None, - FreeInput(_) => panic!(), - MatchExpression(_, _) => None, - } - } - - fn evaluate_binary_operation( - &self, - left: &::ast::parsed::Expression, - op: BinaryOperator, - right: &::ast::parsed::Expression, - ) -> Option { - Some(ast::evaluate_binary_operation( - self.evaluate_expression(left)?, - op, - self.evaluate_expression(right)?, - )) - } - - fn evaluate_unary_operation( - &self, - op: UnaryOperator, - value: &::ast::parsed::Expression, - ) -> Option { - Some(ast::evaluate_unary_operation( - op, - self.evaluate_expression(value)?, - )) - } } pub fn inline_intermediate_polynomials(analyzed: &Analyzed) -> Vec> { From 7235b884846f840dae0e7f8d099552c416a71993 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 9 Oct 2023 16:39:30 +0200 Subject: [PATCH 08/15] Make Identity more flexible. --- ast/src/analyzed/display.rs | 8 ++--- ast/src/analyzed/mod.rs | 20 ++++++------- ast/src/analyzed/visitor.rs | 6 ++-- ast/src/parsed/display.rs | 14 --------- ast/src/parsed/mod.rs | 20 +++++++++---- ast/src/parsed/visitor.rs | 6 ++-- .../json_exporter/expression_counter.rs | 27 ++++++----------- executor/src/witgen/generator.rs | 9 +++--- executor/src/witgen/global_constraints.rs | 6 ++-- executor/src/witgen/identity_processor.rs | 18 ++++++++---- executor/src/witgen/machines/block_machine.rs | 19 ++++++------ .../machines/double_sorted_witness_machine.rs | 11 ++++--- .../witgen/machines/fixed_lookup_machine.rs | 7 +++-- .../src/witgen/machines/machine_extractor.rs | 15 ++++++---- executor/src/witgen/machines/mod.rs | 9 +++--- .../witgen/machines/sorted_witness_machine.rs | 16 +++++----- executor/src/witgen/processor.rs | 13 +++++---- executor/src/witgen/vm_processor.rs | 14 ++++----- halo2/src/circuit_builder.rs | 6 ++-- parser/src/powdr.lalrpop | 2 +- pil_analyzer/src/pil_analyzer.rs | 29 +++++++++---------- 21 files changed, 136 insertions(+), 139 deletions(-) diff --git a/ast/src/analyzed/display.rs b/ast/src/analyzed/display.rs index e7ac24fc9c..6b020f49aa 100644 --- a/ast/src/analyzed/display.rs +++ b/ast/src/analyzed/display.rs @@ -7,8 +7,6 @@ use std::fmt::{Display, Formatter, Result}; use itertools::Itertools; -use crate::parsed::display::format_expressions; - use super::*; impl Display for Analyzed { @@ -102,7 +100,7 @@ impl Display for RepeatedArray { } } -impl Display for Identity { +impl Display for Identity> { fn fmt(&self, f: &mut Formatter<'_>) -> Result { match self.kind { IdentityKind::Polynomial => { @@ -120,7 +118,7 @@ impl Display for Identity { } } -impl Display for SelectedExpressions { +impl Display for SelectedExpressions { fn fmt(&self, f: &mut Formatter<'_>) -> Result { write!( f, @@ -129,7 +127,7 @@ impl Display for SelectedExpressions { .as_ref() .map(|s| format!("{s} ")) .unwrap_or_default(), - format_expressions(&self.expressions) + self.expressions.iter().format(", ") ) } } diff --git a/ast/src/analyzed/mod.rs b/ast/src/analyzed/mod.rs index 20376db860..9c37d3c767 100644 --- a/ast/src/analyzed/mod.rs +++ b/ast/src/analyzed/mod.rs @@ -9,11 +9,11 @@ use std::ops::ControlFlow; use number::DegreeType; -use crate::parsed; use crate::parsed::utils::expr_any; use crate::parsed::visitor::ExpressionVisitable; pub use crate::parsed::BinaryOperator; pub use crate::parsed::UnaryOperator; +use crate::parsed::{self, SelectedExpressions}; #[derive(Debug)] pub enum StatementIdentifier { @@ -29,7 +29,7 @@ pub struct Analyzed { pub constants: HashMap, pub definitions: HashMap>)>, pub public_declarations: HashMap, - pub identities: Vec>, + pub identities: Vec>>, /// The order in which definitions and identities /// appear in the source. pub source_order: Vec, @@ -308,24 +308,26 @@ pub struct PublicDeclaration { } #[derive(Debug, PartialEq, Eq, Clone)] -pub struct Identity { +pub struct Identity { /// The ID is specific to the identity kind. pub id: u64, pub kind: IdentityKind, pub source: SourceRef, /// For a simple polynomial identity, the selector contains /// the actual expression (see expression_for_poly_id). - pub left: SelectedExpressions, - pub right: SelectedExpressions, + pub left: SelectedExpressions, + pub right: SelectedExpressions, } -impl Identity { +impl Identity { /// Returns the expression in case this is a polynomial identity. - pub fn expression_for_poly_id(&self) -> &Expression { + pub fn expression_for_poly_id(&self) -> &Expr { assert_eq!(self.kind, IdentityKind::Polynomial); self.left.selector.as_ref().unwrap() } +} +impl Identity> { pub fn contains_next_ref(&self) -> bool { self.left.contains_next_ref() || self.right.contains_next_ref() } @@ -339,9 +341,7 @@ pub enum IdentityKind { Connect, } -pub type SelectedExpressions = parsed::SelectedExpressions; - -impl SelectedExpressions { +impl SelectedExpressions> { /// @returns true if the expression contains a reference to a next value of a /// (witness or fixed) column pub fn contains_next_ref(&self) -> bool { diff --git a/ast/src/analyzed/visitor.rs b/ast/src/analyzed/visitor.rs index 9d7d2094c8..11ea2d3d3a 100644 --- a/ast/src/analyzed/visitor.rs +++ b/ast/src/analyzed/visitor.rs @@ -50,10 +50,10 @@ impl ExpressionVisitable> for Analyzed { } } -impl ExpressionVisitable> for Identity { +impl> ExpressionVisitable for Identity { fn visit_expressions_mut(&mut self, f: &mut F, o: VisitOrder) -> ControlFlow where - F: FnMut(&mut parsed::Expression) -> ControlFlow, + F: FnMut(&mut Expr) -> ControlFlow, { self.left .selector @@ -67,7 +67,7 @@ impl ExpressionVisitable> for Identity { fn visit_expressions(&self, f: &mut F, o: VisitOrder) -> ControlFlow where - F: FnMut(&parsed::Expression) -> ControlFlow, + F: FnMut(&Expr) -> ControlFlow, { self.left .selector diff --git a/ast/src/parsed/display.rs b/ast/src/parsed/display.rs index ca229465c1..24c16eb5fc 100644 --- a/ast/src/parsed/display.rs +++ b/ast/src/parsed/display.rs @@ -416,20 +416,6 @@ impl Display for FunctionDefinition { } } -impl Display for SelectedExpressions { - fn fmt(&self, f: &mut Formatter<'_>) -> Result { - write!( - f, - "{}{{ {} }}", - self.selector - .as_ref() - .map(|s| format!("{s} ")) - .unwrap_or_default(), - format_expressions(&self.expressions) - ) - } -} - pub fn format_expressions(expressions: &[Expression]) -> String { format!("{}", expressions.iter().format(", ")) } diff --git a/ast/src/parsed/mod.rs b/ast/src/parsed/mod.rs index 3098f99958..d96cf9ab2b 100644 --- a/ast/src/parsed/mod.rs +++ b/ast/src/parsed/mod.rs @@ -28,8 +28,16 @@ pub enum PilStatement { PolynomialConstantDefinition(usize, String, FunctionDefinition), PolynomialCommitDeclaration(usize, Vec>, Option>), PolynomialIdentity(usize, Expression), - PlookupIdentity(usize, SelectedExpressions, SelectedExpressions), - PermutationIdentity(usize, SelectedExpressions, SelectedExpressions), + PlookupIdentity( + usize, + SelectedExpressions>, + SelectedExpressions>, + ), + PermutationIdentity( + usize, + SelectedExpressions>, + SelectedExpressions>, + ), ConnectIdentity(usize, Vec>, Vec>), ConstantDefinition(usize, String, Expression), MacroDefinition( @@ -43,12 +51,12 @@ pub enum PilStatement { } #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)] -pub struct SelectedExpressions> { - pub selector: Option>, - pub expressions: Vec>, +pub struct SelectedExpressions { + pub selector: Option, + pub expressions: Vec, } -impl Default for SelectedExpressions { +impl Default for SelectedExpressions { fn default() -> Self { Self { selector: Default::default(), diff --git a/ast/src/parsed/visitor.rs b/ast/src/parsed/visitor.rs index 626e4187e9..9059daa450 100644 --- a/ast/src/parsed/visitor.rs +++ b/ast/src/parsed/visitor.rs @@ -256,10 +256,10 @@ impl ExpressionVisitable>> for Pi } } -impl ExpressionVisitable> for SelectedExpressions { +impl> ExpressionVisitable for SelectedExpressions { fn visit_expressions_mut(&mut self, f: &mut F, o: VisitOrder) -> ControlFlow where - F: FnMut(&mut Expression) -> ControlFlow, + F: FnMut(&mut Expr) -> ControlFlow, { self.selector .as_mut() @@ -270,7 +270,7 @@ impl ExpressionVisitable> for SelectedExpressions(&self, f: &mut F, o: VisitOrder) -> ControlFlow where - F: FnMut(&Expression) -> ControlFlow, + F: FnMut(&Expr) -> ControlFlow, { self.selector .as_ref() diff --git a/backend/src/pilstark/json_exporter/expression_counter.rs b/backend/src/pilstark/json_exporter/expression_counter.rs index 3b4c4d772e..af161c53d5 100644 --- a/backend/src/pilstark/json_exporter/expression_counter.rs +++ b/backend/src/pilstark/json_exporter/expression_counter.rs @@ -1,8 +1,11 @@ use std::collections::HashMap; -use ast::analyzed::{ - Analyzed, Expression, Identity, PolynomialType, PublicDeclaration, SelectedExpressions, - StatementIdentifier, Symbol, SymbolKind, +use ast::{ + analyzed::{ + Analyzed, Identity, PolynomialType, PublicDeclaration, StatementIdentifier, Symbol, + SymbolKind, + }, + parsed::SelectedExpressions, }; /// Computes expression IDs for each intermediate polynomial. @@ -32,7 +35,7 @@ trait ExpressionCounter { fn expression_count(&self) -> usize; } -impl ExpressionCounter for Identity { +impl ExpressionCounter for Identity { fn expression_count(&self) -> usize { self.left.expression_count() + self.right.expression_count() } @@ -50,20 +53,8 @@ impl ExpressionCounter for PublicDeclaration { } } -impl ExpressionCounter for SelectedExpressions { +impl ExpressionCounter for SelectedExpressions { fn expression_count(&self) -> usize { - self.selector.expression_count() + self.expressions.expression_count() - } -} - -impl ExpressionCounter for Vec> { - fn expression_count(&self) -> usize { - self.len() - } -} - -impl ExpressionCounter for Option> { - fn expression_count(&self) -> usize { - (self.is_some()).into() + self.selector.is_some() as usize + self.expressions.len() } } diff --git a/executor/src/witgen/generator.rs b/executor/src/witgen/generator.rs index 5121b88b7f..c511e4420c 100644 --- a/executor/src/witgen/generator.rs +++ b/executor/src/witgen/generator.rs @@ -1,4 +1,5 @@ -use ast::analyzed::{Identity, IdentityKind, PolyID, PolynomialReference, SelectedExpressions}; +use ast::analyzed::{Expression, Identity, IdentityKind, PolyID, PolynomialReference}; +use ast::parsed::SelectedExpressions; use number::FieldElement; use std::collections::{HashMap, HashSet}; @@ -15,7 +16,7 @@ use super::{EvalResult, FixedData, MutableState}; pub struct Generator<'a, T: FieldElement> { fixed_data: &'a FixedData<'a, T>, - identities: Vec<&'a Identity>, + identities: Vec<&'a Identity>>, witnesses: HashSet, global_range_constraints: WitnessColumnMap>>, data: Vec>, @@ -28,7 +29,7 @@ impl<'a, T: FieldElement> Machine<'a, T> for Generator<'a, T> { _fixed_lookup: &mut FixedLookup, _kind: IdentityKind, _left: &[AffineExpression<&'a PolynomialReference, T>], - _right: &'a SelectedExpressions, + _right: &'a SelectedExpressions>, _machines: Machines<'a, '_, T>, ) -> Option> { unimplemented!() @@ -50,7 +51,7 @@ impl<'a, T: FieldElement> Machine<'a, T> for Generator<'a, T> { impl<'a, T: FieldElement> Generator<'a, T> { pub fn new( fixed_data: &'a FixedData<'a, T>, - identities: &[&'a Identity], + identities: &[&'a Identity>], witnesses: HashSet, global_range_constraints: &WitnessColumnMap>>, ) -> Self { diff --git a/executor/src/witgen/global_constraints.rs b/executor/src/witgen/global_constraints.rs index 146e48e66f..c20ea99da3 100644 --- a/executor/src/witgen/global_constraints.rs +++ b/executor/src/witgen/global_constraints.rs @@ -35,7 +35,7 @@ impl<'a, T: FieldElement> RangeConstraintSet<&PolynomialReference, T> pub struct GlobalConstraints<'a, T: FieldElement> { pub known_witness_constraints: WitnessColumnMap>>, - pub retained_identities: Vec<&'a Identity>, + pub retained_identities: Vec<&'a Identity>>, } /// Determines global constraints on witness and fixed columns. @@ -44,7 +44,7 @@ pub struct GlobalConstraints<'a, T: FieldElement> { /// TODO at some point, we should check that they still hold. pub fn determine_global_constraints<'a, T: FieldElement>( fixed_data: &'a FixedData, - identities: Vec<&'a Identity>, + identities: Vec<&'a Identity>>, ) -> GlobalConstraints<'a, T> { let mut known_constraints = BTreeMap::new(); // For these columns, we know that they are not only constrained to those bits @@ -134,7 +134,7 @@ fn process_fixed_column(fixed: &[T]) -> Option<(RangeConstraint /// no further information than the range constraint. fn propagate_constraints( mut known_constraints: BTreeMap>, - identity: &Identity, + identity: &Identity>, full_span: &BTreeSet, ) -> (BTreeMap>, bool) { let mut remove = false; diff --git a/executor/src/witgen/identity_processor.rs b/executor/src/witgen/identity_processor.rs index e308230fdc..76579baf41 100644 --- a/executor/src/witgen/identity_processor.rs +++ b/executor/src/witgen/identity_processor.rs @@ -1,6 +1,9 @@ use std::{collections::HashMap, sync::Mutex}; -use ast::analyzed::{Expression, Identity, IdentityKind, PolynomialReference, SelectedExpressions}; +use ast::{ + analyzed::{Expression, Identity, IdentityKind, PolynomialReference}, + parsed::SelectedExpressions, +}; use itertools::{Either, Itertools}; use lazy_static::lazy_static; use number::FieldElement; @@ -86,7 +89,7 @@ impl<'a, 'b, T: FieldElement> IdentityProcessor<'a, 'b, T> { /// Returns the updates. pub fn process_identity( &mut self, - identity: &'a Identity, + identity: &'a Identity>, rows: &RowPair<'_, 'a, T>, ) -> EvalResult<'a, T> { let result = match identity.kind { @@ -106,7 +109,7 @@ impl<'a, 'b, T: FieldElement> IdentityProcessor<'a, 'b, T> { fn process_polynomial_identity( &self, - identity: &'a Identity, + identity: &'a Identity>, rows: &RowPair, ) -> EvalResult<'a, T> { match rows.evaluate(identity.expression_for_poly_id()) { @@ -117,7 +120,7 @@ impl<'a, 'b, T: FieldElement> IdentityProcessor<'a, 'b, T> { fn process_plookup( &mut self, - identity: &'a Identity, + identity: &'a Identity>, rows: &RowPair<'_, 'a, T>, ) -> EvalResult<'a, T> { if let Some(left_selector) = &identity.left.selector { @@ -187,7 +190,7 @@ impl<'a, 'b, T: FieldElement> IdentityProcessor<'a, 'b, T> { pub fn process_link( &mut self, left: &[AffineExpression<&'a PolynomialReference, T>], - right: &'a SelectedExpressions, + right: &'a SelectedExpressions>, current_rows: &RowPair<'_, 'a, T>, ) -> EvalResult<'a, T> { // sanity check that the right hand side selector is active @@ -246,7 +249,10 @@ lazy_static! { Mutex::new(Default::default()); } -fn report_identity_solving(identity: &Identity, result: &EvalResult) { +fn report_identity_solving( + identity: &Identity>, + result: &EvalResult, +) { let success = result.as_ref().map(|r| r.is_complete()).unwrap_or_default() as u64; let mut stat = STATISTICS.lock().unwrap(); stat.entry((identity.id, identity.kind)) diff --git a/executor/src/witgen/machines/block_machine.rs b/executor/src/witgen/machines/block_machine.rs index 1425b8e302..b58e5924b5 100644 --- a/executor/src/witgen/machines/block_machine.rs +++ b/executor/src/witgen/machines/block_machine.rs @@ -12,9 +12,8 @@ use crate::witgen::Constraints; use crate::witgen::{ machines::Machine, range_constraints::RangeConstraint, EvalError, EvalValue, IncompleteCause, }; -use ast::analyzed::{ - Expression, Identity, IdentityKind, PolyID, PolynomialReference, SelectedExpressions, -}; +use ast::analyzed::{Expression, Identity, IdentityKind, PolyID, PolynomialReference}; +use ast::parsed::SelectedExpressions; use number::{DegreeType, FieldElement}; enum ProcessResult<'a, T: FieldElement> { @@ -39,9 +38,9 @@ pub struct BlockMachine<'a, T: FieldElement> { block_size: usize, /// The right-hand side of the connecting identity, needed to identify /// when this machine is responsible. - selected_expressions: SelectedExpressions, + selected_expressions: SelectedExpressions>, /// The internal identities - identities: Vec<&'a Identity>, + identities: Vec<&'a Identity>>, /// The row factory row_factory: RowFactory<'a, T>, /// The data of the machine. @@ -57,8 +56,8 @@ pub struct BlockMachine<'a, T: FieldElement> { impl<'a, T: FieldElement> BlockMachine<'a, T> { pub fn try_new( fixed_data: &'a FixedData<'a, T>, - connecting_identities: &[&'a Identity], - identities: &[&'a Identity], + connecting_identities: &[&'a Identity>], + identities: &[&'a Identity>], witness_cols: &HashSet, global_range_constraints: &WitnessColumnMap>>, ) -> Option { @@ -140,7 +139,7 @@ impl<'a, T: FieldElement> Machine<'a, T> for BlockMachine<'a, T> { fixed_lookup: &'b mut FixedLookup, kind: IdentityKind, left: &[AffineExpression<&'a PolynomialReference, T>], - right: &'a SelectedExpressions, + right: &'a SelectedExpressions>, machines: Machines<'a, 'b, T>, ) -> Option> { if *right != self.selected_expressions || kind != IdentityKind::Plookup { @@ -252,7 +251,7 @@ impl<'a, T: FieldElement> BlockMachine<'a, T> { &mut self, fixed_lookup: &'b mut FixedLookup, left: &[AffineExpression<&'a PolynomialReference, T>], - right: &'a SelectedExpressions, + right: &'a SelectedExpressions>, machines: Machines<'a, 'b, T>, ) -> EvalResult<'a, T> { log::trace!("Start processing block machine '{}'", self.name()); @@ -351,7 +350,7 @@ impl<'a, T: FieldElement> BlockMachine<'a, T> { &self, identity_processor: &mut IdentityProcessor<'a, 'b, T>, left: &[AffineExpression<&'a PolynomialReference, T>], - right: &'a SelectedExpressions, + right: &'a SelectedExpressions>, sequence_iterator: &mut ProcessingSequenceIterator, ) -> Result, EvalError> { // Make the block two rows larger than the block size, it includes the last row of the previous block diff --git a/executor/src/witgen/machines/double_sorted_witness_machine.rs b/executor/src/witgen/machines/double_sorted_witness_machine.rs index f414e4e195..bf2313f91a 100644 --- a/executor/src/witgen/machines/double_sorted_witness_machine.rs +++ b/executor/src/witgen/machines/double_sorted_witness_machine.rs @@ -1,6 +1,7 @@ use std::collections::{BTreeMap, HashMap, HashSet}; use std::iter::once; +use ast::parsed::SelectedExpressions; use itertools::Itertools; use num_traits::Zero; @@ -12,9 +13,7 @@ use crate::witgen::{EvalResult, FixedData}; use crate::witgen::{EvalValue, IncompleteCause}; use number::{DegreeType, FieldElement}; -use ast::analyzed::{ - Expression, Identity, IdentityKind, PolyID, PolynomialReference, Reference, SelectedExpressions, -}; +use ast::analyzed::{Expression, Identity, IdentityKind, PolyID, PolynomialReference, Reference}; /// TODO make this generic @@ -43,7 +42,7 @@ impl DoubleSortedWitnesses { pub fn try_new( fixed_data: &FixedData, - _identities: &[&Identity], + _identities: &[&Identity>], witness_cols: &HashSet, ) -> Option { // get the namespaces and column names @@ -100,7 +99,7 @@ impl<'a, T: FieldElement> Machine<'a, T> for DoubleSortedWitnesses { _fixed_lookup: &mut FixedLookup, kind: IdentityKind, left: &[AffineExpression<&'a PolynomialReference, T>], - right: &'a SelectedExpressions, + right: &'a SelectedExpressions>, _machines: Machines<'a, '_, T>, ) -> Option> { if kind != IdentityKind::Permutation @@ -174,7 +173,7 @@ impl DoubleSortedWitnesses { fn process_plookup_internal<'a>( &mut self, left: &[AffineExpression<&'a PolynomialReference, T>], - right: &SelectedExpressions, + right: &SelectedExpressions>, ) -> EvalResult<'a, T> { // We blindly assume the lookup is of the form // OP { ADDR, STEP, X } is m_is_write { m_addr, m_step, m_value } diff --git a/executor/src/witgen/machines/fixed_lookup_machine.rs b/executor/src/witgen/machines/fixed_lookup_machine.rs index 7e5e8c95b3..fc06f9fad3 100644 --- a/executor/src/witgen/machines/fixed_lookup_machine.rs +++ b/executor/src/witgen/machines/fixed_lookup_machine.rs @@ -2,7 +2,8 @@ use std::collections::{BTreeMap, HashMap, HashSet}; use std::mem; use std::num::NonZeroUsize; -use ast::analyzed::{Identity, IdentityKind, PolyID, PolynomialReference, SelectedExpressions}; +use ast::analyzed::{Expression, Identity, IdentityKind, PolyID, PolynomialReference}; +use ast::parsed::SelectedExpressions; use itertools::Itertools; use number::FieldElement; @@ -162,7 +163,7 @@ pub struct FixedLookup { impl FixedLookup { pub fn try_new( _fixed_data: &FixedData, - identities: &[&Identity], + identities: &[&Identity>], witness_names: &HashSet<&str>, ) -> Option { if identities.is_empty() && witness_names.is_empty() { @@ -177,7 +178,7 @@ impl FixedLookup { fixed_data: &FixedData, kind: IdentityKind, left: &[AffineExpression<&'b PolynomialReference, T>], - right: &'b SelectedExpressions, + right: &'b SelectedExpressions>, ) -> Option> { // This is a matching machine if it is a plookup and the RHS is fully constant. if kind != IdentityKind::Plookup diff --git a/executor/src/witgen/machines/machine_extractor.rs b/executor/src/witgen/machines/machine_extractor.rs index 7363524e21..bf7236f058 100644 --- a/executor/src/witgen/machines/machine_extractor.rs +++ b/executor/src/witgen/machines/machine_extractor.rs @@ -9,15 +9,16 @@ use super::KnownMachine; use crate::witgen::{ column_map::WitnessColumnMap, generator::Generator, range_constraints::RangeConstraint, }; -use ast::analyzed::{Expression, Identity, IdentityKind, PolyID, Reference, SelectedExpressions}; +use ast::analyzed::{Expression, Identity, IdentityKind, PolyID, Reference}; use ast::parsed::visitor::ExpressionVisitable; +use ast::parsed::SelectedExpressions; use itertools::Itertools; use number::FieldElement; pub struct ExtractionOutput<'a, T: FieldElement> { pub fixed_lookup: FixedLookup, pub machines: Vec>, - pub base_identities: Vec<&'a Identity>, + pub base_identities: Vec<&'a Identity>>, pub base_witnesses: HashSet, } @@ -26,7 +27,7 @@ pub struct ExtractionOutput<'a, T: FieldElement> { /// that are not "internal" to the machines. pub fn split_out_machines<'a, T: FieldElement>( fixed: &'a FixedData<'a, T>, - identities: Vec<&'a Identity>, + identities: Vec<&'a Identity>>, global_range_constraints: &WitnessColumnMap>>, ) -> ExtractionOutput<'a, T> { let fixed_lookup = FixedLookup::try_new(fixed, &[], &Default::default()).unwrap(); @@ -140,7 +141,7 @@ pub fn split_out_machines<'a, T: FieldElement>( fn all_row_connected_witnesses( mut witnesses: HashSet, all_witnesses: &HashSet, - identities: &[&Identity], + identities: &[&Identity>], ) -> HashSet { loop { let count = witnesses.len(); @@ -173,7 +174,7 @@ fn all_row_connected_witnesses( } /// Extracts all references to names from an identity. -pub fn refs_in_identity(identity: &Identity) -> HashSet { +pub fn refs_in_identity(identity: &Identity>) -> HashSet { let mut refs: HashSet = Default::default(); identity.pre_visit_expressions(&mut |expr| { ref_of_expression(expr).map(|id| refs.insert(id)); @@ -182,7 +183,9 @@ pub fn refs_in_identity(identity: &Identity) -> HashSet { } /// Extracts all references to names from selected expressions. -pub fn refs_in_selected_expressions(selexpr: &SelectedExpressions) -> HashSet { +pub fn refs_in_selected_expressions( + selexpr: &SelectedExpressions>, +) -> HashSet { let mut refs: HashSet = Default::default(); selexpr.pre_visit_expressions(&mut |expr| { ref_of_expression(expr).map(|id| refs.insert(id)); diff --git a/executor/src/witgen/machines/mod.rs b/executor/src/witgen/machines/mod.rs index 761404f118..4bb0f555e9 100644 --- a/executor/src/witgen/machines/mod.rs +++ b/executor/src/witgen/machines/mod.rs @@ -1,14 +1,15 @@ use std::collections::HashMap; -use ast::analyzed::IdentityKind; +use ast::analyzed::Expression; use ast::analyzed::PolynomialReference; -use ast::analyzed::SelectedExpressions; +use ast::parsed::SelectedExpressions; use number::FieldElement; use self::block_machine::BlockMachine; use self::double_sorted_witness_machine::DoubleSortedWitnesses; pub use self::fixed_lookup_machine::FixedLookup; use self::sorted_witness_machine::SortedWitnesses; +use ast::analyzed::IdentityKind; use super::affine_expression::AffineExpression; use super::generator::Generator; @@ -36,7 +37,7 @@ pub trait Machine<'a, T: FieldElement>: Send + Sync { fixed_lookup: &'b mut FixedLookup, kind: IdentityKind, left: &[AffineExpression<&'a PolynomialReference, T>], - right: &'a SelectedExpressions, + right: &'a SelectedExpressions>, machines: Machines<'a, 'b, T>, ) -> Option>; @@ -72,7 +73,7 @@ impl<'a, T: FieldElement> Machine<'a, T> for KnownMachine<'a, T> { fixed_lookup: &'b mut FixedLookup, kind: IdentityKind, left: &[AffineExpression<&'a PolynomialReference, T>], - right: &'a SelectedExpressions, + right: &'a SelectedExpressions>, machines: Machines<'a, 'b, T>, ) -> Option> { self.get() diff --git a/executor/src/witgen/machines/sorted_witness_machine.rs b/executor/src/witgen/machines/sorted_witness_machine.rs index 0e3fc3857b..92e7de27f9 100644 --- a/executor/src/witgen/machines/sorted_witness_machine.rs +++ b/executor/src/witgen/machines/sorted_witness_machine.rs @@ -1,5 +1,6 @@ use std::collections::{BTreeMap, HashMap, HashSet}; +use ast::parsed::SelectedExpressions; use itertools::Itertools; use super::super::affine_expression::AffineExpression; @@ -12,9 +13,7 @@ use crate::witgen::{ symbolic_evaluator::SymbolicEvaluator, }; use crate::witgen::{EvalValue, IncompleteCause}; -use ast::analyzed::{ - Expression, Identity, IdentityKind, PolyID, PolynomialReference, Reference, SelectedExpressions, -}; +use ast::analyzed::{Expression, Identity, IdentityKind, PolyID, PolynomialReference, Reference}; use number::FieldElement; /// A machine that can support a lookup in a set of columns that are sorted @@ -34,7 +33,7 @@ pub struct SortedWitnesses { impl SortedWitnesses { pub fn try_new( fixed_data: &FixedData, - identities: &[&Identity], + identities: &[&Identity>], witnesses: &HashSet, ) -> Option { if identities.len() != 1 { @@ -58,7 +57,10 @@ impl SortedWitnesses { } } -fn check_identity(fixed_data: &FixedData, id: &Identity) -> Option { +fn check_identity( + fixed_data: &FixedData, + id: &Identity>, +) -> Option { // Looking for NOTLAST { A' - A } in { POSITIVE } if id.kind != IdentityKind::Plookup || id.right.selector.is_some() @@ -125,7 +127,7 @@ impl<'a, T: FieldElement> Machine<'a, T> for SortedWitnesses { _fixed_lookup: &mut FixedLookup, kind: IdentityKind, left: &[AffineExpression<&'a PolynomialReference, T>], - right: &'a SelectedExpressions, + right: &'a SelectedExpressions>, _machines: Machines<'a, '_, T>, ) -> Option> { if kind != IdentityKind::Plookup || right.selector.is_some() { @@ -182,7 +184,7 @@ impl SortedWitnesses { &mut self, fixed_data: &FixedData, left: &[AffineExpression<&'a PolynomialReference, T>], - right: &SelectedExpressions, + right: &SelectedExpressions>, rhs: Vec<&PolynomialReference>, ) -> EvalResult<'a, T> { let key_index = rhs diff --git a/executor/src/witgen/processor.rs b/executor/src/witgen/processor.rs index 5d8a88fdaf..6e067314d1 100644 --- a/executor/src/witgen/processor.rs +++ b/executor/src/witgen/processor.rs @@ -1,6 +1,9 @@ use std::{collections::HashSet, marker::PhantomData}; -use ast::analyzed::{Identity, PolyID, PolynomialReference, SelectedExpressions}; +use ast::{ + analyzed::{Expression, Identity, PolyID, PolynomialReference}, + parsed::SelectedExpressions, +}; use number::FieldElement; use crate::witgen::Constraint; @@ -25,11 +28,11 @@ pub struct OuterQuery<'a, T: FieldElement> { /// This will be mutated while processing the block. left: Left<'a, T>, /// The right-hand side of the outer query. - right: &'a SelectedExpressions, + right: &'a SelectedExpressions>, } impl<'a, T: FieldElement> OuterQuery<'a, T> { - pub fn new(left: Left<'a, T>, right: &'a SelectedExpressions) -> Self { + pub fn new(left: Left<'a, T>, right: &'a SelectedExpressions>) -> Self { Self { left, right } } } @@ -46,7 +49,7 @@ pub struct Processor<'a, 'b, 'c, T: FieldElement, CalldataAvailable> { /// The rows that are being processed. data: Vec>, /// The list of identities - identities: &'c [&'a Identity], + identities: &'c [&'a Identity>], /// The identity processor identity_processor: &'c mut IdentityProcessor<'a, 'b, T>, /// The fixed data (containing information about all columns) @@ -65,7 +68,7 @@ impl<'a, 'b, 'c, T: FieldElement> Processor<'a, 'b, 'c, T, WithoutCalldata> { row_offset: u64, data: Vec>, identity_processor: &'c mut IdentityProcessor<'a, 'b, T>, - identities: &'c [&'a Identity], + identities: &'c [&'a Identity>], fixed_data: &'a FixedData<'a, T>, row_factory: RowFactory<'a, T>, witness_cols: &'c HashSet, diff --git a/executor/src/witgen/vm_processor.rs b/executor/src/witgen/vm_processor.rs index 39d493f8a4..25542d9eb9 100644 --- a/executor/src/witgen/vm_processor.rs +++ b/executor/src/witgen/vm_processor.rs @@ -1,4 +1,4 @@ -use ast::analyzed::{Identity, IdentityKind, PolyID, PolynomialReference}; +use ast::analyzed::{Expression, Identity, IdentityKind, PolyID, PolynomialReference}; use itertools::Itertools; use number::{DegreeType, FieldElement}; use parser_util::lines::indent; @@ -23,18 +23,18 @@ enum ProcessingPhase { /// A list of identities with a flag whether it is complete. struct CompletableIdentities<'a, T: FieldElement> { - identities_with_complete: Vec<(&'a Identity, bool)>, + identities_with_complete: Vec<(&'a Identity>, bool)>, } impl<'a, T: FieldElement> CompletableIdentities<'a, T> { - fn new(identities: impl Iterator>) -> Self { + fn new(identities: impl Iterator>>) -> Self { Self { identities_with_complete: identities.map(|identity| (identity, false)).collect(), } } /// Yields immutable references to the identity and mutable references to the complete flag. - fn iter_mut(&mut self) -> impl Iterator, &mut bool)> { + fn iter_mut(&mut self) -> impl Iterator>, &mut bool)> { self.identities_with_complete .iter_mut() .map(|(identity, complete)| (*identity, complete)) @@ -47,10 +47,10 @@ pub struct VmProcessor<'a, T: FieldElement> { fixed_data: &'a FixedData<'a, T>, /// The subset of identities that contains a reference to the next row /// (precomputed once for performance reasons) - identities_with_next_ref: Vec<&'a Identity>, + identities_with_next_ref: Vec<&'a Identity>>, /// The subset of identities that does not contain a reference to the next row /// (precomputed once for performance reasons) - identities_without_next_ref: Vec<&'a Identity>, + identities_without_next_ref: Vec<&'a Identity>>, data: Vec>, last_report: DegreeType, last_report_time: Instant, @@ -59,7 +59,7 @@ pub struct VmProcessor<'a, T: FieldElement> { impl<'a, T: FieldElement> VmProcessor<'a, T> { pub fn new( fixed_data: &'a FixedData<'a, T>, - identities: &[&'a Identity], + identities: &[&'a Identity>], witnesses: HashSet, data: Vec>, ) -> Self { diff --git a/halo2/src/circuit_builder.rs b/halo2/src/circuit_builder.rs index b1c12c4ef8..fcfc5e6abb 100644 --- a/halo2/src/circuit_builder.rs +++ b/halo2/src/circuit_builder.rs @@ -1,4 +1,4 @@ -use ast::parsed::BinaryOperator; +use ast::parsed::{BinaryOperator, SelectedExpressions}; use num_bigint::BigUint; use polyexen::expr::{ColumnKind, ColumnQuery, Expr, PlonkVar}; use polyexen::plaf::backends::halo2::PlafH2Circuit; @@ -6,7 +6,7 @@ use polyexen::plaf::{ ColumnFixed, ColumnWitness, Columns, Info, Lookup, Plaf, Poly, Shuffle, Witness, }; -use ast::analyzed::{Analyzed, Expression, IdentityKind, Reference, SelectedExpressions}; +use ast::analyzed::{Analyzed, Expression, IdentityKind, Reference}; use num_traits::One; use number::{BigInt, FieldElement}; @@ -89,7 +89,7 @@ pub(crate) fn analyzed_to_circuit( // build Plaf polys. ------------------------------------------------------------------------- - let apply_selectors_to_set = |set: &SelectedExpressions| { + let apply_selectors_to_set = |set: &SelectedExpressions>| { let selector = set .selector .clone() diff --git a/parser/src/powdr.lalrpop b/parser/src/powdr.lalrpop index 632b13ec79..723b90bd32 100644 --- a/parser/src/powdr.lalrpop +++ b/parser/src/powdr.lalrpop @@ -140,7 +140,7 @@ PlookupIdentity: PilStatement = { <@L> "in" => PilStatement::PlookupIdentity(<>) } -SelectedExpressions: SelectedExpressions = { +SelectedExpressions: SelectedExpressions> = { "{" "}" => SelectedExpressions{<>}, Expression => SelectedExpressions{selector: None, expressions: vec![<>]}, } diff --git a/pil_analyzer/src/pil_analyzer.rs b/pil_analyzer/src/pil_analyzer.rs index 6763ea2eff..0eb2d548b8 100644 --- a/pil_analyzer/src/pil_analyzer.rs +++ b/pil_analyzer/src/pil_analyzer.rs @@ -7,14 +7,14 @@ use analysis::MacroExpander; use ast::parsed::visitor::ExpressionVisitable; use ast::parsed::{ self, ArrayExpression, ArrayLiteral, FunctionDefinition, LambdaExpression, MatchArm, - MatchPattern, PilStatement, PolynomialName, + MatchPattern, PilStatement, PolynomialName, SelectedExpressions, }; use number::{DegreeType, FieldElement}; use ast::analyzed::{ Analyzed, Expression, FunctionValueDefinition, Identity, IdentityKind, PolynomialReference, - PolynomialType, PublicDeclaration, Reference, RepeatedArray, SelectedExpressions, SourceRef, - StatementIdentifier, Symbol, SymbolKind, + PolynomialType, PublicDeclaration, Reference, RepeatedArray, SourceRef, StatementIdentifier, + Symbol, SymbolKind, }; use crate::evaluator::Evaluator; @@ -39,7 +39,7 @@ struct PILAnalyzer { constants: HashMap, definitions: HashMap>)>, public_declarations: HashMap, - identities: Vec>, + identities: Vec>>, /// The order in which definitions and identities /// appear in the source. source_order: Vec, @@ -513,8 +513,8 @@ impl<'a, T: FieldElement> ExpressionProcessor<'a, T> { pub fn process_selected_expression( &mut self, - expr: ::ast::parsed::SelectedExpressions, - ) -> SelectedExpressions { + expr: SelectedExpressions>, + ) -> SelectedExpressions> { SelectedExpressions { selector: expr.selector.map(|e| self.process_expression(e)), expressions: self.process_expressions(expr.expressions), @@ -550,18 +550,15 @@ impl<'a, T: FieldElement> ExpressionProcessor<'a, T> { } } - pub fn process_expressions( - &mut self, - exprs: Vec<::ast::parsed::Expression>, - ) -> Vec> { + pub fn process_expressions(&mut self, exprs: Vec>) -> Vec> { exprs .into_iter() .map(|e| self.process_expression(e)) .collect() } - pub fn process_expression(&mut self, expr: ::ast::parsed::Expression) -> Expression { - use ast::parsed::Expression as PExpression; + pub fn process_expression(&mut self, expr: parsed::Expression) -> Expression { + use parsed::Expression as PExpression; match expr { PExpression::Constant(name) => Expression::Constant(name), PExpression::Reference(poly) => { @@ -675,7 +672,9 @@ impl<'a, T: FieldElement> ExpressionProcessor<'a, T> { } } -pub fn inline_intermediate_polynomials(analyzed: &Analyzed) -> Vec> { +pub fn inline_intermediate_polynomials( + analyzed: &Analyzed, +) -> Vec>> { substitute_intermediate( analyzed.identities.clone(), &analyzed @@ -697,9 +696,9 @@ pub fn inline_intermediate_polynomials(analyzed: &Analyzed) -> Vec( - identities: impl IntoIterator>, + identities: impl IntoIterator>>, intermediate_polynomials: &HashMap>, -) -> Vec> { +) -> Vec>> { identities .into_iter() .scan(HashMap::default(), |cache, mut identity| { From 01e6e0f6fd92d8ab1cc99466086b74c2d4aef86d Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Tue, 10 Oct 2023 09:23:47 +0000 Subject: [PATCH 09/15] Witgen Refactoring: Pull out wrapping handling into Generator --- executor/src/witgen/generator.rs | 83 +++++++++++-- executor/src/witgen/processor.rs | 4 + executor/src/witgen/rows.rs | 33 +++++- executor/src/witgen/sequence_iterator.rs | 3 +- .../src/witgen/symbolic_witness_evaluator.rs | 8 +- executor/src/witgen/vm_processor.rs | 112 ++++++++---------- 6 files changed, 163 insertions(+), 80 deletions(-) diff --git a/executor/src/witgen/generator.rs b/executor/src/witgen/generator.rs index c511e4420c..a211b1781d 100644 --- a/executor/src/witgen/generator.rs +++ b/executor/src/witgen/generator.rs @@ -1,16 +1,20 @@ use ast::analyzed::{Expression, Identity, IdentityKind, PolyID, PolynomialReference}; use ast::parsed::SelectedExpressions; -use number::FieldElement; +use number::{DegreeType, FieldElement}; use std::collections::{HashMap, HashSet}; +use crate::witgen::rows::CellValue; + use super::affine_expression::AffineExpression; use super::column_map::WitnessColumnMap; -use super::identity_processor::Machines; +use super::identity_processor::{IdentityProcessor, Machines}; use super::machines::Machine; +use super::processor::Processor; use super::range_constraints::RangeConstraint; use super::machines::FixedLookup; use super::rows::{transpose_rows, Row, RowFactory}; +use super::sequence_iterator::{DefaultSequenceIterator, ProcessingSequenceIterator}; use super::vm_processor::VmProcessor; use super::{EvalResult, FixedData, MutableState}; @@ -68,20 +72,85 @@ impl<'a, T: FieldElement> Generator<'a, T> { where Q: FnMut(&str) -> Option + Send + Sync, { - // For now, run the VM Processor on an empty block of the size of the degree - // In the future, we'll instantate a processor for each block and then stitch them together here. + let first_row = self.compute_partial_first_row(mutable_state); + self.data = self.process(first_row, mutable_state); + self.fix_first_row(); + } + + /// Runs the solver on the row pair (degree - 1, 0) in order to partially compute the first + /// row from identities like `pc' = (1 - first_step') * <...>`. + fn compute_partial_first_row(&self, mutable_state: &mut MutableState<'a, T, Q>) -> Row<'a, T> + where + Q: FnMut(&str) -> Option + Send + Sync, + { + // Use `Processor` + `DefaultSequenceIterator` using a "block size" of 0. Because `Processor` + // expects `data` to include the row before and after the block, this means we'll run the + // solver on exactly one row pair. + // Note that using `Processor` instead of `VmProcessor` is more convenient here because + // it does not assert that the row is "complete" afterwards (i.e., that all identities + // are satisfied assuming 0 for unknown values). + let mut identity_processor = IdentityProcessor::new( + self.fixed_data, + &mut mutable_state.fixed_lookup, + mutable_state.machines.iter_mut().into(), + ); let row_factory = RowFactory::new(self.fixed_data, self.global_range_constraints.clone()); - let default_row = row_factory.fresh_row(); - let data = vec![default_row; self.fixed_data.degree as usize]; + let data = vec![row_factory.fresh_row(); 2]; + let mut processor = Processor::new( + self.fixed_data.degree - 1, + data, + &mut identity_processor, + &self.identities, + self.fixed_data, + row_factory, + &self.witnesses, + ); + let mut sequence_iterator = ProcessingSequenceIterator::Default( + DefaultSequenceIterator::new(0, self.identities.len(), None), + ); + processor.solve(&mut sequence_iterator).unwrap(); + let first_row = processor.finish().remove(1); + first_row + } + + fn process( + &self, + first_row: Row<'a, T>, + mutable_state: &mut MutableState<'a, T, Q>, + ) -> Vec> + where + Q: FnMut(&str) -> Option + Send + Sync, + { + log::trace!("{}", first_row.render("first row", false, &self.witnesses)); + let data = vec![first_row]; + let row_factory = RowFactory::new(self.fixed_data, self.global_range_constraints.clone()); let mut processor = VmProcessor::new( self.fixed_data, &self.identities, self.witnesses.clone(), data, + row_factory, ); processor.run(mutable_state); + processor.finish() + } + + /// At the end of the solving algorithm, we'll have computed the first row twice + /// (as row 0 and as row ). This function merges the two versions. + fn fix_first_row(&mut self) { + assert_eq!(self.data.len() as DegreeType, self.fixed_data.degree + 1); - self.data = processor.finish(); + let last_row = self.data.pop().unwrap(); + self.data[0] = WitnessColumnMap::from(self.data[0].values().zip(last_row.values()).map( + |(cell1, cell2)| match (&cell1.value, &cell2.value) { + (CellValue::Known(v1), CellValue::Known(v2)) => { + assert_eq!(v1, v2); + cell1.clone() + } + (CellValue::Known(_), _) => cell1.clone(), + _ => cell2.clone(), + }, + )); } } diff --git a/executor/src/witgen/processor.rs b/executor/src/witgen/processor.rs index 6e067314d1..3e169d59ff 100644 --- a/executor/src/witgen/processor.rs +++ b/executor/src/witgen/processor.rs @@ -102,6 +102,10 @@ impl<'a, 'b, 'c, T: FieldElement> Processor<'a, 'b, 'c, T, WithoutCalldata> { witness_cols: self.witness_cols, } } + + pub fn finish(self) -> Vec> { + self.data + } } impl<'a, 'b, T: FieldElement> Processor<'a, 'b, '_, T, WithCalldata> { diff --git a/executor/src/witgen/rows.rs b/executor/src/witgen/rows.rs index 05c2dab829..e011184d88 100644 --- a/executor/src/witgen/rows.rs +++ b/executor/src/witgen/rows.rs @@ -307,12 +307,13 @@ pub enum UnknownStrategy { /// to return for a given [PolynomialReference]. pub struct RowPair<'row, 'a, T: FieldElement> { pub current: &'row Row<'a, T>, - pub next: &'row Row<'a, T>, + pub next: Option<&'row Row<'a, T>>, pub current_row_index: DegreeType, fixed_data: &'a FixedData<'a, T>, unknown_strategy: UnknownStrategy, } impl<'row, 'a, T: FieldElement> RowPair<'row, 'a, T> { + /// Creates a new row pair. pub fn new( current: &'row Row<'a, T>, next: &'row Row<'a, T>, @@ -322,17 +323,39 @@ impl<'row, 'a, T: FieldElement> RowPair<'row, 'a, T> { ) -> Self { Self { current, - next, + next: Some(next), current_row_index, fixed_data, unknown_strategy, } } + /// Creates a new row pair from a single row, setting the next row to None. + pub fn from_single_row( + current: &'row Row<'a, T>, + current_row_index: DegreeType, + fixed_data: &'a FixedData<'a, T>, + unknown_strategy: UnknownStrategy, + ) -> Self { + Self { + current, + next: None, + current_row_index, + fixed_data, + unknown_strategy, + } + } + + /// Gets the cell corresponding to the given polynomial reference. + /// + /// # Panics + /// Panics if the next row is accessed but the row pair has been constructed with + /// [RowPair::from_single_row]. fn get_cell(&self, poly: &PolynomialReference) -> &Cell { - match poly.next { - false => &self.current[&poly.poly_id()], - true => &self.next[&poly.poly_id()], + match (poly.next, self.next.as_ref()) { + (false, _) => &self.current[&poly.poly_id()], + (true, Some(next)) => &next[&poly.poly_id()], + (true, None) => panic!("Tried to access next row, but it is not available."), } } diff --git a/executor/src/witgen/sequence_iterator.rs b/executor/src/witgen/sequence_iterator.rs index 6a9b6bc863..e763202b8c 100644 --- a/executor/src/witgen/sequence_iterator.rs +++ b/executor/src/witgen/sequence_iterator.rs @@ -38,7 +38,6 @@ const MAX_ROUNDS_PER_ROW_DELTA: usize = 100; impl DefaultSequenceIterator { pub fn new(block_size: usize, identities_count: usize, outer_query_row: Option) -> Self { - assert!(block_size >= 1); let max_row = block_size as i64 - 1; DefaultSequenceIterator { identities_count, @@ -113,7 +112,7 @@ impl DefaultSequenceIterator { pub fn next(&mut self) -> Option { self.update_state(); - if self.cur_row_delta_index == self.row_deltas.len() { + if self.cur_row_delta_index == self.row_deltas.len() || self.identities_count == 0 { // Done! return None; } diff --git a/executor/src/witgen/symbolic_witness_evaluator.rs b/executor/src/witgen/symbolic_witness_evaluator.rs index fd2f151ef9..93753f1444 100644 --- a/executor/src/witgen/symbolic_witness_evaluator.rs +++ b/executor/src/witgen/symbolic_witness_evaluator.rs @@ -46,12 +46,8 @@ where } else { // Constant polynomial (or something else) let values = self.fixed_data.fixed_cols[&poly.poly_id()].values; - let row = if poly.next { - let degree = values.len() as DegreeType; - (self.row + 1) % degree - } else { - self.row - }; + let row = + if poly.next { self.row + 1 } else { self.row } % (values.len() as DegreeType); Ok(values[row as usize].into()) } } diff --git a/executor/src/witgen/vm_processor.rs b/executor/src/witgen/vm_processor.rs index 25542d9eb9..7f843c215b 100644 --- a/executor/src/witgen/vm_processor.rs +++ b/executor/src/witgen/vm_processor.rs @@ -11,16 +11,9 @@ use crate::witgen::rows::RowUpdater; use super::query_processor::QueryProcessor; -use super::rows::{Row, RowPair, UnknownStrategy}; +use super::rows::{Row, RowFactory, RowPair, UnknownStrategy}; use super::{EvalError, EvalResult, EvalValue, FixedData, MutableState}; -/// Phase in which [VmProcessor::compute_row] is called. -#[derive(Debug, PartialEq)] -enum ProcessingPhase { - Initialization, - Regular, -} - /// A list of identities with a flag whether it is complete. struct CompletableIdentities<'a, T: FieldElement> { identities_with_complete: Vec<(&'a Identity>, bool)>, @@ -54,6 +47,7 @@ pub struct VmProcessor<'a, T: FieldElement> { data: Vec>, last_report: DegreeType, last_report_time: Instant, + row_factory: RowFactory<'a, T>, } impl<'a, T: FieldElement> VmProcessor<'a, T> { @@ -62,6 +56,7 @@ impl<'a, T: FieldElement> VmProcessor<'a, T> { identities: &[&'a Identity>], witnesses: HashSet, data: Vec>, + row_factory: RowFactory<'a, T>, ) -> Self { let (identities_with_next, identities_without_next): (Vec<_>, Vec<_>) = identities .iter() @@ -73,6 +68,7 @@ impl<'a, T: FieldElement> VmProcessor<'a, T> { identities_with_next_ref: identities_with_next, identities_without_next_ref: identities_without_next, data, + row_factory, last_report: 0, last_report_time: Instant::now(), } @@ -82,26 +78,19 @@ impl<'a, T: FieldElement> VmProcessor<'a, T> { self.data } - fn last_row(&self) -> DegreeType { - self.fixed_data.degree - 1 - } - + /// Starting out with a single row, iteratively append rows until we have degree + 1 rows + /// (i.e., we have the first row twice). pub fn run(&mut self, mutable_state: &mut MutableState<'a, T, Q>) where Q: FnMut(&str) -> Option + Send + Sync, { - // For identities like `pc' = (1 - first_step') * <...>`, we need to process the last - // row before processing the first row. - self.compute_row( - self.last_row(), - ProcessingPhase::Initialization, - mutable_state, - ); + assert!(self.data.len() == 1); // Are we in an infinite loop and can just re-use the old values? let mut looping_period = None; let mut loop_detection_log_level = log::Level::Info; - for row_index in 0..self.fixed_data.degree as DegreeType { + let rows_left = self.fixed_data.degree + 1; + for row_index in 0..rows_left { self.maybe_log_performance(row_index); // Check if we are in a loop. if looping_period.is_none() && row_index % 100 == 0 && row_index > 0 { @@ -126,10 +115,16 @@ impl<'a, T: FieldElement> VmProcessor<'a, T> { loop_detection_log_level = log::Level::Debug; } } - if looping_period.is_none() { - self.compute_row(row_index, ProcessingPhase::Regular, mutable_state); + // Note that we exit one iteration early in the non-loop case, + // because ensure_has_next_row() + compute_row() will already + // add and compute some values for the next row as well. + if looping_period.is_none() && row_index != rows_left - 1 { + self.ensure_has_next_row(row_index); + self.compute_row(row_index, mutable_state); }; } + + assert_eq!(self.data.len() as DegreeType, self.fixed_data.degree + 1); } /// Checks if the last rows are repeating and returns the period. @@ -152,16 +147,18 @@ impl<'a, T: FieldElement> VmProcessor<'a, T> { // Returns a reference to a given row fn row(&self, row_index: DegreeType) -> &Row<'a, T> { - let row_index = (row_index + self.fixed_data.degree) % self.fixed_data.degree; &self.data[row_index as usize] } - fn compute_row( - &mut self, - row_index: DegreeType, - phase: ProcessingPhase, - mutable_state: &mut MutableState<'a, T, Q>, - ) where + fn ensure_has_next_row(&mut self, row_index: DegreeType) { + assert!(self.data.len() as DegreeType > row_index); + if row_index == self.data.len() as DegreeType - 1 { + self.data.push(self.row_factory.fresh_row()); + } + } + + fn compute_row(&mut self, row_index: DegreeType, mutable_state: &mut MutableState<'a, T, Q>) + where Q: FnMut(&str) -> Option + Send + Sync, { log::trace!("Row: {}", row_index); @@ -202,30 +199,23 @@ impl<'a, T: FieldElement> VmProcessor<'a, T> { // Check that the computed row is "final" by asserting that all unknown values can // be set to 0. - // This check is skipped in the initialization phase (run on the last row), - // because its only purpose is to transfer values to the first row, - // not to finalize the last row. - if phase == ProcessingPhase::Regular { - log::trace!( - " Checking that remaining identities hold when unknown values are set to 0" - ); + log::trace!(" Checking that remaining identities hold when unknown values are set to 0"); + self.process_identities( + row_index, + &mut identities_without_next_ref, + UnknownStrategy::Zero, + &mut identity_processor, + ) + .and_then(|_| { self.process_identities( row_index, - &mut identities_without_next_ref, + &mut identities_with_next_ref, UnknownStrategy::Zero, &mut identity_processor, ) - .and_then(|_| { - self.process_identities( - row_index, - &mut identities_with_next_ref, - UnknownStrategy::Zero, - &mut identity_processor, - ) - }) - .map_err(|e| self.report_failure_and_panic_underconstrained(row_index, e)) - .unwrap(); - } + }) + .map_err(|e| self.report_failure_and_panic_underconstrained(row_index, e)) + .unwrap(); log::trace!( "{}", @@ -349,13 +339,7 @@ impl<'a, T: FieldElement> VmProcessor<'a, T> { updates: &EvalValue<&PolynomialReference, T>, source_name: impl Fn() -> String, ) -> bool { - let (before, after) = if row_index == self.last_row() { - // Last row is current, first row is next - let (after, before) = self.data.split_at_mut(row_index as usize); - (before, after) - } else { - self.data.split_at_mut(row_index as usize + 1) - }; + let (before, after) = self.data.split_at_mut(row_index as usize + 1); let current = before.last_mut().unwrap(); let next = after.first_mut().unwrap(); let mut row_updater = RowUpdater::new(current, next, row_index); @@ -447,12 +431,21 @@ impl<'a, T: FieldElement> VmProcessor<'a, T> { && self.check_row_pair(row_index, &proposed_row, true, &mut identity_processor); if constraints_valid { - self.data[row_index as usize] = proposed_row; + if row_index as usize == self.data.len() - 1 { + // We might already have added the row in [VmProcessor::ensure_has_next_row] + self.data[row_index as usize] = proposed_row; + } else { + // If the previous row was also added by [VmProcessor::try_propose_row], we won't have an entry + // for the current row yet. + assert_eq!(row_index as usize, self.data.len()); + self.data.push(proposed_row); + } } else { // Note that we never update the next row if proposing a row succeeds (the happy path). // If it doesn't, we re-run compute_next_row on the previous row in order to // correctly forward-propagate values via next references. - self.compute_row(row_index - 1, ProcessingPhase::Regular, mutable_state); + self.ensure_has_next_row(row_index - 1); + self.compute_row(row_index - 1, mutable_state); } constraints_valid } @@ -476,10 +469,9 @@ impl<'a, T: FieldElement> VmProcessor<'a, T> { ), // Check whether identities without a reference to the next row are satisfied // when applied to the proposed row. - // Note that we also provide the next row here, but it is not used. - false => RowPair::new( + // Because we never access the next row, we can use [RowPair::from_single_row] here. + false => RowPair::from_single_row( proposed_row, - self.row(row_index + 1), row_index, self.fixed_data, UnknownStrategy::Zero, From 48d6de72bd356bbf1f04f089efc016901fe1ac13 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 10 Oct 2023 15:50:50 +0200 Subject: [PATCH 10/15] Visitor for FunctionValueDefinition --- ast/src/analyzed/visitor.rs | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/ast/src/analyzed/visitor.rs b/ast/src/analyzed/visitor.rs index 9d7d2094c8..317a86a2b5 100644 --- a/ast/src/analyzed/visitor.rs +++ b/ast/src/analyzed/visitor.rs @@ -79,3 +79,35 @@ impl ExpressionVisitable> for Identity { .try_for_each(move |item| item.visit_expressions(f, o)) } } + +impl ExpressionVisitable> for FunctionValueDefinition { + fn visit_expressions_mut(&mut self, f: &mut F, o: VisitOrder) -> ControlFlow + where + F: FnMut(&mut Expression) -> ControlFlow, + { + match self { + FunctionValueDefinition::Mapping(e) + | FunctionValueDefinition::Query(e) + | FunctionValueDefinition::Expression(e) => e.visit_expressions_mut(f, o), + FunctionValueDefinition::Array(array) => array + .iter_mut() + .flat_map(|a| a.pattern.iter_mut()) + .try_for_each(move |item| item.visit_expressions_mut(f, o)), + } + } + + fn visit_expressions(&self, f: &mut F, o: VisitOrder) -> ControlFlow + where + F: FnMut(&Expression) -> ControlFlow, + { + match self { + FunctionValueDefinition::Mapping(e) + | FunctionValueDefinition::Query(e) + | FunctionValueDefinition::Expression(e) => e.visit_expressions(f, o), + FunctionValueDefinition::Array(array) => array + .iter() + .flat_map(|a| a.pattern().iter()) + .try_for_each(move |item| item.visit_expressions(f, o)), + } + } +} From aff7ff1c579d86cb6e71870cbf22a5f6a530954d Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 29 Sep 2023 12:23:50 +0200 Subject: [PATCH 11/15] Do not treat constants specially during parsing. --- asm_to_pil/src/vm_to_constrained.rs | 1 - ast/src/parsed/display.rs | 1 - ast/src/parsed/mod.rs | 2 -- ast/src/parsed/visitor.rs | 2 -- backend/src/pilstark/json_exporter/mod.rs | 5 ----- executor/src/witgen/expression_evaluator.rs | 3 --- halo2/src/circuit_builder.rs | 3 --- parser/src/powdr.lalrpop | 2 +- pil_analyzer/src/evaluator.rs | 1 - pil_analyzer/src/pil_analyzer.rs | 3 +-- pilopt/src/lib.rs | 6 ++---- 11 files changed, 4 insertions(+), 25 deletions(-) diff --git a/asm_to_pil/src/vm_to_constrained.rs b/asm_to_pil/src/vm_to_constrained.rs index 19ed3916e4..995fbddc79 100644 --- a/asm_to_pil/src/vm_to_constrained.rs +++ b/asm_to_pil/src/vm_to_constrained.rs @@ -575,7 +575,6 @@ impl ASMPILConverter { value: Expression, ) -> Vec<(T, AffineExpressionComponent)> { match value { - Expression::Constant(_) => panic!(), Expression::PublicReference(_) => panic!(), Expression::FunctionCall(_) => panic!(), Expression::Reference(reference) => { diff --git a/ast/src/parsed/display.rs b/ast/src/parsed/display.rs index 24c16eb5fc..e4cf0706d5 100644 --- a/ast/src/parsed/display.rs +++ b/ast/src/parsed/display.rs @@ -423,7 +423,6 @@ pub fn format_expressions(expressions: &[Expression Display for Expression { fn fmt(&self, f: &mut Formatter<'_>) -> Result { match self { - Expression::Constant(name) => write!(f, "{name}"), Expression::Reference(reference) => write!(f, "{reference}"), Expression::PublicReference(name) => write!(f, ":{name}"), Expression::Number(value) => write!(f, "{value}"), diff --git a/ast/src/parsed/mod.rs b/ast/src/parsed/mod.rs index d96cf9ab2b..472cb8795f 100644 --- a/ast/src/parsed/mod.rs +++ b/ast/src/parsed/mod.rs @@ -67,8 +67,6 @@ impl Default for SelectedExpressions { #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)] pub enum Expression> { - /// Reference to a constant, "%ConstantName" - Constant(String), Reference(Ref), PublicReference(String), Number(T), diff --git a/ast/src/parsed/visitor.rs b/ast/src/parsed/visitor.rs index 9059daa450..c28829617f 100644 --- a/ast/src/parsed/visitor.rs +++ b/ast/src/parsed/visitor.rs @@ -114,7 +114,6 @@ impl ExpressionVisitable> for Expression { } match self { Expression::Reference(_) - | Expression::Constant(_) | Expression::PublicReference(_) | Expression::Number(_) | Expression::String(_) => {} @@ -152,7 +151,6 @@ impl ExpressionVisitable> for Expression { } match self { Expression::Reference(_) - | Expression::Constant(_) | Expression::PublicReference(_) | Expression::Number(_) | Expression::String(_) => {} diff --git a/backend/src/pilstark/json_exporter/mod.rs b/backend/src/pilstark/json_exporter/mod.rs index 333db0a29a..3de8c7506d 100644 --- a/backend/src/pilstark/json_exporter/mod.rs +++ b/backend/src/pilstark/json_exporter/mod.rs @@ -235,11 +235,6 @@ impl<'a, T: FieldElement> Exporter<'a, T> { /// returns the degree and the JSON value (intermediate polynomial IDs) fn expression_to_json(&self, expr: &Expression) -> (u32, StarkyExpr) { match expr { - Expression::Constant(name) => { - panic!( - "Constant {name} was not inlined. optimize_constants needs to be run at least." - ) - } Expression::Reference(analyzed::Reference::Poly(reference)) => { self.polynomial_reference_to_json(reference) } diff --git a/executor/src/witgen/expression_evaluator.rs b/executor/src/witgen/expression_evaluator.rs index 30c12cf156..e45b97b3b4 100644 --- a/executor/src/witgen/expression_evaluator.rs +++ b/executor/src/witgen/expression_evaluator.rs @@ -37,9 +37,6 @@ where // @TODO if we iterate on processing the constraints in the same row, // we could store the simplified values. match expr { - Expression::Constant(name) => { - panic!("Constants should have been replaced, but {name} is still there.") - } Expression::Reference(Reference::Poly(poly)) => self.variables.value(poly), Expression::Number(n) => Ok((*n).into()), Expression::BinaryOperation(left, op, right) => { diff --git a/halo2/src/circuit_builder.rs b/halo2/src/circuit_builder.rs index fcfc5e6abb..71a0ab2ef6 100644 --- a/halo2/src/circuit_builder.rs +++ b/halo2/src/circuit_builder.rs @@ -256,9 +256,6 @@ fn expression_2_expr(cd: &CircuitData, expr: &Expression) _ => unimplemented!("{:?}", expr), } } - Expression::Constant(name) => { - panic!("Constant {name} was not inlined. optimize_constants needs to be run at least.") - } _ => unimplemented!("{:?}", expr), } diff --git a/parser/src/powdr.lalrpop b/parser/src/powdr.lalrpop index 723b90bd32..d96319a8d4 100644 --- a/parser/src/powdr.lalrpop +++ b/parser/src/powdr.lalrpop @@ -462,7 +462,7 @@ UnaryOp: UnaryOperator = { Term: Box> = { FunctionCall => Box::new(Expression::FunctionCall(<>)), - ConstantIdentifier => Box::new(Expression::Constant(<>)), + ConstantIdentifier => Box::new(Expression::Reference(PolynomialReference::new(<>).single().local().current())), ShiftedPolynomialReference => Box::new(Expression::Reference(<>)), PublicReference => Box::new(Expression::PublicReference(<>)), FieldElement => Box::new(Expression::Number(<>)), diff --git a/pil_analyzer/src/evaluator.rs b/pil_analyzer/src/evaluator.rs index 357315f1aa..a88505b069 100644 --- a/pil_analyzer/src/evaluator.rs +++ b/pil_analyzer/src/evaluator.rs @@ -32,7 +32,6 @@ pub struct Evaluator<'a, T> { impl<'a, T: FieldElement> Evaluator<'a, T> { pub fn evaluate(&self, expr: &Expression) -> Result { match expr { - Expression::Constant(name) => Ok(self.constants[name]), Expression::Reference(Reference::LocalVar(i, _name)) => Ok(self.variables[*i as usize]), Expression::Reference(Reference::Poly(poly)) => { if !poly.next && poly.index.is_none() { diff --git a/pil_analyzer/src/pil_analyzer.rs b/pil_analyzer/src/pil_analyzer.rs index 0eb2d548b8..6f173a191f 100644 --- a/pil_analyzer/src/pil_analyzer.rs +++ b/pil_analyzer/src/pil_analyzer.rs @@ -51,7 +51,7 @@ struct PILAnalyzer { macro_expander: MacroExpander, } -impl From> for Analyzed { +impl From> for Analyzed { fn from( PILAnalyzer { constants, @@ -560,7 +560,6 @@ impl<'a, T: FieldElement> ExpressionProcessor<'a, T> { pub fn process_expression(&mut self, expr: parsed::Expression) -> Expression { use parsed::Expression as PExpression; match expr { - PExpression::Constant(name) => Expression::Constant(name), PExpression::Reference(poly) => { if poly.namespace().is_none() && self.local_variables.contains_key(poly.name()) { let id = self.local_variables[poly.name()]; diff --git a/pilopt/src/lib.rs b/pilopt/src/lib.rs index 65138db22d..9b37436c5a 100644 --- a/pilopt/src/lib.rs +++ b/pilopt/src/lib.rs @@ -46,16 +46,14 @@ pub fn optimize_constants(mut pil_file: Analyzed) -> Analyze /// Inlines references to symbols with a single constant value. fn inline_constant_values(pil_file: &mut Analyzed) { let constants = &pil_file.constants.clone(); - pil_file.post_visit_expressions_mut(&mut |e| match e { - Expression::Reference(Reference::Poly(poly)) => { + pil_file.post_visit_expressions_mut(&mut |e| { + if let Expression::Reference(Reference::Poly(poly)) = e { if !poly.next && poly.index.is_none() { if let Some(value) = constants.get(&poly.name) { *e = Expression::Number(*value) } } } - Expression::Constant(name) => *e = Expression::Number(constants[name]), - _ => {} }); } From 1e81bb36ac477dc2079fd6125e74a2afb9917021 Mon Sep 17 00:00:00 2001 From: chriseth Date: Sat, 30 Sep 2023 00:11:53 +0200 Subject: [PATCH 12/15] Move constants to generic definitions. --- ast/src/analyzed/display.rs | 33 ++++---- ast/src/analyzed/mod.rs | 28 ++++--- backend/src/pilstark/json_exporter/mod.rs | 18 +++-- executor/src/constant_evaluator/mod.rs | 2 - pil_analyzer/src/evaluator.rs | 38 +++++---- pil_analyzer/src/pil_analyzer.rs | 93 +++++++++++++---------- pilopt/src/lib.rs | 3 +- 7 files changed, 126 insertions(+), 89 deletions(-) diff --git a/ast/src/analyzed/display.rs b/ast/src/analyzed/display.rs index 6b020f49aa..93dc9f1f45 100644 --- a/ast/src/analyzed/display.rs +++ b/ast/src/analyzed/display.rs @@ -11,28 +11,25 @@ use super::*; impl Display for Analyzed { fn fmt(&self, f: &mut Formatter<'_>) -> Result { - for (name, value) in &self.constants { - writeln!(f, "constant {name} = {value};")?; - } - - let mut namespace = "Global".to_string(); + let mut current_namespace = "Global".to_string(); let mut update_namespace = |name: &str, degree: DegreeType, f: &mut Formatter<'_>| { - if let Some(dot) = name.find('.') { - if name[..dot] != namespace { - namespace = name[..dot].to_string(); - writeln!(f, "namespace {namespace}({degree});")?; + let new_name = if let Some(dot) = name.find('.') { + if name[..dot] != current_namespace { + current_namespace = name[..dot].to_string(); + writeln!(f, "namespace {current_namespace}({degree});")?; } - Ok(name[dot + 1..].to_string()) + &name[dot + 1..] } else { - Ok(name.to_string()) - } + name + }; + Ok((new_name.to_string(), ¤t_namespace != "Global")) }; for statement in &self.source_order { match statement { StatementIdentifier::Definition(name) => { let (symbol, definition) = &self.definitions[name]; - let name = update_namespace(name, symbol.degree, f)?; + let (name, is_local) = update_namespace(name, symbol.degree, f)?; match symbol.kind { SymbolKind::Poly(poly_type) => { let kind = match &poly_type { @@ -47,6 +44,14 @@ impl Display for Analyzed { writeln!(f, ";")? } } + SymbolKind::Constant() => { + let indentation = if is_local { " " } else { "" }; + writeln!( + f, + "{indentation}constant {name}{};", + definition.as_ref().unwrap() + )?; + } SymbolKind::Other() => { write!(f, " let {name}")?; if let Some(value) = definition { @@ -59,7 +64,7 @@ impl Display for Analyzed { StatementIdentifier::PublicDeclaration(name) => { let decl = &self.public_declarations[name]; // TODO we do not know the degree of the namespace here. - let name = update_namespace(&decl.name, 0, f)?; + let (name, _) = update_namespace(&decl.name, 0, f)?; writeln!( f, " public {name} = {}({});", diff --git a/ast/src/analyzed/mod.rs b/ast/src/analyzed/mod.rs index 9c37d3c767..0b4e37c0e9 100644 --- a/ast/src/analyzed/mod.rs +++ b/ast/src/analyzed/mod.rs @@ -25,8 +25,6 @@ pub enum StatementIdentifier { #[derive(Debug)] pub struct Analyzed { - /// Constants are not namespaced! - pub constants: HashMap, pub definitions: HashMap>)>, pub public_declarations: HashMap, pub identities: Vec>>, @@ -141,7 +139,9 @@ impl Analyzed { let mut names_to_remove: HashSet = Default::default(); self.definitions.retain(|name, (poly, _def)| { - if to_remove.contains(&(poly as &Symbol).into()) { + if matches!(poly.kind, SymbolKind::Poly(_)) + && to_remove.contains(&(poly as &Symbol).into()) + { names_to_remove.insert(name.clone()); false } else { @@ -157,15 +157,18 @@ impl Analyzed { true }); self.definitions.values_mut().for_each(|(poly, _def)| { - let poly_id = PolyID::from(poly as &Symbol); - assert!(!to_remove.contains(&poly_id)); - poly.id = replacements[&poly_id].id; + if matches!(poly.kind, SymbolKind::Poly(_)) { + let poly_id = PolyID::from(poly as &Symbol); + assert!(!to_remove.contains(&poly_id)); + poly.id = replacements[&poly_id].id; + } }); self.pre_visit_expressions_mut(&mut |expr| { if let Expression::Reference(Reference::Poly(poly)) = expr { - let poly_id = poly.poly_id.unwrap(); - assert!(!to_remove.contains(&poly_id)); - poly.poly_id = Some(replacements[&poly_id]); + poly.poly_id = poly.poly_id.map(|poly_id| { + assert!(!to_remove.contains(&poly_id)); + replacements[&poly_id] + }); } }); } @@ -240,11 +243,16 @@ impl Symbol { } } +/// The "kind" of a symbol. In the future, this will be mostly +/// replaced by its type. #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] pub enum SymbolKind { /// Fixed, witness or intermediate polynomial Poly(PolynomialType), - /// Other symbol, could be a constant, depends on the type. + /// A constant value. + Constant(), + /// Other symbol, depends on the type. + /// Examples include functions not of the type "int -> fe". Other(), } diff --git a/backend/src/pilstark/json_exporter/mod.rs b/backend/src/pilstark/json_exporter/mod.rs index 3de8c7506d..76fd48573c 100644 --- a/backend/src/pilstark/json_exporter/mod.rs +++ b/backend/src/pilstark/json_exporter/mod.rs @@ -145,6 +145,7 @@ fn symbol_kind_to_json_string(k: SymbolKind) -> &'static str { match k { SymbolKind::Poly(poly_type) => polynomial_type_to_json_string(poly_type), SymbolKind::Other() => panic!("Cannot translate \"other\" symbol to json."), + SymbolKind::Constant() => unreachable!(), } } @@ -183,12 +184,15 @@ impl<'a, T: FieldElement> Exporter<'a, T> { self.analyzed .definitions .iter() - .map(|(name, (symbol, _value))| { - let id = if symbol.kind == SymbolKind::Poly(PolynomialType::Intermediate) { - self.intermediate_poly_expression_ids[&symbol.id] - } else { - symbol.id - }; + .filter_map(|(name, (symbol, _value))| { + let id = match symbol.kind { + SymbolKind::Poly(PolynomialType::Intermediate) => { + Some(self.intermediate_poly_expression_ids[&symbol.id]) + } + SymbolKind::Poly(_) => Some(symbol.id), + SymbolKind::Other() | SymbolKind::Constant() => None, + }?; + let out = Reference { polType: None, type_: symbol_kind_to_json_string(symbol.kind).to_string(), @@ -198,7 +202,7 @@ impl<'a, T: FieldElement> Exporter<'a, T> { elementType: None, len: symbol.length.map(|l| l as usize), }; - (name.clone(), out) + Some((name.clone(), out)) }) .collect::>() } diff --git a/executor/src/constant_evaluator/mod.rs b/executor/src/constant_evaluator/mod.rs index bfeab5e137..6bf3cd6c6a 100644 --- a/executor/src/constant_evaluator/mod.rs +++ b/executor/src/constant_evaluator/mod.rs @@ -41,7 +41,6 @@ fn generate_values( .into_par_iter() .map(|i| { Evaluator { - constants: &analyzed.constants, definitions: &analyzed.definitions, variables: &[i.into()], function_cache: other_constants, @@ -52,7 +51,6 @@ fn generate_values( .collect(), FunctionValueDefinition::Array(values) => { let evaluator = Evaluator { - constants: &analyzed.constants, definitions: &analyzed.definitions, variables: &[], function_cache: other_constants, diff --git a/pil_analyzer/src/evaluator.rs b/pil_analyzer/src/evaluator.rs index a88505b069..ec6651dd32 100644 --- a/pil_analyzer/src/evaluator.rs +++ b/pil_analyzer/src/evaluator.rs @@ -1,7 +1,7 @@ use std::collections::HashMap; use ast::{ - analyzed::{Analyzed, Expression, FunctionValueDefinition, Reference, Symbol}, + analyzed::{Analyzed, Expression, FunctionValueDefinition, Reference, Symbol, SymbolKind}, evaluate_binary_operation, evaluate_unary_operation, parsed::{FunctionCall, MatchArm, MatchPattern}, }; @@ -13,7 +13,6 @@ pub fn evaluate_expression( expression: &Expression, ) -> Result { Evaluator { - constants: &analyzed.constants, definitions: &analyzed.definitions, function_cache: &Default::default(), variables: &[], @@ -21,8 +20,26 @@ pub fn evaluate_expression( .evaluate(expression) } +/// Returns a HashMap of all symbols that have a constant single value. +pub fn compute_constants(analyzed: &Analyzed) -> HashMap { + analyzed + .definitions + .iter() + .filter_map(|(name, (symbol, value))| { + (symbol.kind == SymbolKind::Constant()).then(|| { + let Some(FunctionValueDefinition::Expression(value)) = value else { + panic!() + }; + ( + name.to_owned(), + evaluate_expression(analyzed, value).unwrap(), + ) + }) + }) + .collect() +} + pub struct Evaluator<'a, T> { - pub constants: &'a HashMap, pub definitions: &'a HashMap>)>, /// Contains full value tables of functions (columns) we already evaluated. pub function_cache: &'a HashMap<&'a str, Vec>, @@ -35,17 +52,10 @@ impl<'a, T: FieldElement> Evaluator<'a, T> { Expression::Reference(Reference::LocalVar(i, _name)) => Ok(self.variables[*i as usize]), Expression::Reference(Reference::Poly(poly)) => { if !poly.next && poly.index.is_none() { - let name = poly.name.to_owned(); - if let Some(value) = self.constants.get(&name) { - Ok(*value) - } else { - let (_, value) = &self.definitions[&name]; - match value { - Some(FunctionValueDefinition::Expression(value)) => { - self.evaluate(value) - } - _ => Err("Cannot evaluate function values".to_string()), - } + let (_, value) = &self.definitions[&poly.name.to_string()]; + match value { + Some(FunctionValueDefinition::Expression(value)) => self.evaluate(value), + _ => Err("Cannot evaluate function-typed values".to_string()), } } else { Err("Cannot evaluate arrays or next references.".to_string()) diff --git a/pil_analyzer/src/pil_analyzer.rs b/pil_analyzer/src/pil_analyzer.rs index 6f173a191f..39f3ddb203 100644 --- a/pil_analyzer/src/pil_analyzer.rs +++ b/pil_analyzer/src/pil_analyzer.rs @@ -35,8 +35,6 @@ pub fn process_pil_file_contents(contents: &str) -> Analyzed struct PILAnalyzer { namespace: String, polynomial_degree: DegreeType, - /// Constants are not namespaced! - constants: HashMap, definitions: HashMap>)>, public_declarations: HashMap, identities: Vec>>, @@ -54,7 +52,6 @@ struct PILAnalyzer { impl From> for Analyzed { fn from( PILAnalyzer { - constants, definitions, public_declarations, identities, @@ -66,9 +63,7 @@ impl From> for Analyzed { .iter() .map(|(name, (poly, _))| (name.clone(), poly.clone())) .collect::>(); - let constant_names = constants.keys().cloned().collect::>(); let mut result = Self { - constants, definitions, public_declarations, identities, @@ -78,13 +73,13 @@ impl From> for Analyzed { let poly = ids .get(&reference.name) .unwrap_or_else(|| panic!("Column {} not found.", reference.name)); - reference.poly_id = Some(poly.into()); + if let SymbolKind::Poly(_) = &poly.kind { + reference.poly_id = Some(poly.into()); + } }; result.pre_visit_expressions_mut(&mut |e| { if let Expression::Reference(Reference::Poly(reference)) = e { - if !constant_names.contains(&reference.name) { - assign_id(reference); - } + assign_id(reference); } }); result @@ -103,6 +98,7 @@ impl PILAnalyzer { SymbolKind::Poly(PolynomialType::Committed), SymbolKind::Poly(PolynomialType::Constant), SymbolKind::Poly(PolynomialType::Intermediate), + SymbolKind::Constant(), SymbolKind::Other(), ] .into_iter() @@ -196,8 +192,18 @@ impl PILAnalyzer { Some(definition), ); } - PilStatement::ConstantDefinition(_start, name, value) => { - self.handle_constant_definition(name, self.evaluate_expression(value).unwrap()) + PilStatement::ConstantDefinition(start, name, value) => { + // Check it is a constant. + if let Err(err) = self.evaluate_expression(value.clone()) { + panic!("Could not evaluate constant: {name} = {value}: {err}"); + } + self.handle_polynomial_definition( + self.to_source_ref(start), + name, + None, + SymbolKind::Constant(), + Some(FunctionDefinition::Expression(value)), + ); } PilStatement::LetStatement(start, name, value) => { self.handle_generic_definition(start, name, value) @@ -256,19 +262,20 @@ impl PILAnalyzer { ); } _ => { - if let Ok(constant) = self.evaluate_expression(value.clone()) { + let symbol_kind = if self.evaluate_expression(value.clone()).is_ok() { // Value evaluates to a constant number => treat it as a constant - self.handle_constant_definition(name.to_string(), constant); + SymbolKind::Constant() } else { // Otherwise, treat it as "generic definition" - self.handle_polynomial_definition( - self.to_source_ref(start), - name, - None, - SymbolKind::Other(), - Some(FunctionDefinition::Expression(value)), - ); - } + SymbolKind::Other() + }; + self.handle_polynomial_definition( + self.to_source_ref(start), + name, + None, + symbol_kind, + Some(FunctionDefinition::Expression(value)), + ); } } } @@ -375,7 +382,12 @@ impl PILAnalyzer { let counter = self.symbol_counters.get_mut(&symbol_kind).unwrap(); let id = *counter; *counter += length.unwrap_or(1); - let absolute_name = self.namespaced(&name); + let absolute_name = if symbol_kind == SymbolKind::Constant() { + // Constants are not namespaced. + name.to_owned() + } else { + self.prepend_current_namespace(&name) + }; let symbol = Symbol { id, source, @@ -391,6 +403,7 @@ impl PILAnalyzer { assert!(!have_array_size); assert!( symbol_kind == SymbolKind::Other() + || symbol_kind == SymbolKind::Constant() || symbol_kind == SymbolKind::Poly(PolynomialType::Intermediate) ); FunctionValueDefinition::Expression(self.process_expression(expr)) @@ -453,11 +466,6 @@ impl PILAnalyzer { .push(StatementIdentifier::PublicDeclaration(name)); } - fn handle_constant_definition(&mut self, name: String, value: T) { - let is_new = self.constants.insert(name.to_string(), value).is_none(); - assert!(is_new, "Constant {name} was defined twice."); - } - fn dispense_id(&mut self, kind: IdentityKind) -> u64 { let cnt = self.identity_counter.entry(kind).or_default(); let id = *cnt; @@ -465,12 +473,12 @@ impl PILAnalyzer { id } - pub fn namespaced(&self, name: &str) -> String { - self.namespaced_ref(&None, name) + fn prepend_current_namespace(&self, name: &str) -> String { + format!("{}.{name}", self.namespace) } - fn namespaced_ref(&self, namespace: &Option, name: &str) -> String { - if name.starts_with('%') || (namespace.is_none() && self.constants.contains_key(name)) { + pub fn namespaced_ref_to_absolute(&self, namespace: &Option, name: &str) -> String { + if name.starts_with('%') || self.definitions.contains_key(&name.to_string()) { assert!(namespace.is_none()); // Constants are not namespaced name.to_string() @@ -481,7 +489,6 @@ impl PILAnalyzer { fn evaluate_expression(&self, expr: ::ast::parsed::Expression) -> Result { Evaluator { - constants: &self.constants, definitions: &self.definitions, function_cache: &Default::default(), variables: &[], @@ -594,7 +601,7 @@ impl<'a, T: FieldElement> ExpressionProcessor<'a, T> { Expression::UnaryOperation(op, Box::new(self.process_expression(*value))) } PExpression::FunctionCall(c) => Expression::FunctionCall(parsed::FunctionCall { - id: self.analyzer.namespaced(&c.id), + id: self.analyzer.namespaced_ref_to_absolute(&None, &c.id), arguments: self.process_expressions(c.arguments), }), PExpression::MatchExpression(scrutinee, arms) => Expression::MatchExpression( @@ -651,7 +658,9 @@ impl<'a, T: FieldElement> ExpressionProcessor<'a, T> { .as_ref() .map(|i| self.analyzer.evaluate_expression(*i.clone()).unwrap()) .map(|i| i.to_degree()); - let name = self.analyzer.namespaced_ref(poly.namespace(), poly.name()); + let name = self + .analyzer + .namespaced_ref_to_absolute(poly.namespace(), poly.name()); PolynomialReference { name, poly_id: None, @@ -851,18 +860,20 @@ namespace T(65536); #[test] fn let_definitions() { - let input = r#"namespace N(65536); + let input = r#"constant %r = 65536; +namespace N(%r); let x; - let t = |i| i + 1; - let z = 7; - let other = [1, 2]; + let z = 2; + let t = |i| i + z; + let other = [1, z]; let other_fun = |i, j| (i + 7, (|k| k - i)); "#; - let expected = r#"constant z = 7; + let expected = r#"constant %r = 65536; namespace N(65536); col witness x; - col fixed t(i) { (i + 1) }; - let other = [1, 2]; + constant z = 2; + col fixed t(i) { (i + z) }; + let other = [1, z]; let other_fun = |i, j| ((i + 7), |k| (k - i)); "#; let formatted = process_pil_file_contents::(input).to_string(); diff --git a/pilopt/src/lib.rs b/pilopt/src/lib.rs index 9b37436c5a..b99acb82ff 100644 --- a/pilopt/src/lib.rs +++ b/pilopt/src/lib.rs @@ -13,6 +13,7 @@ use ast::parsed::visitor::ExpressionVisitable; use ast::parsed::UnaryOperator; use ast::{evaluate_binary_operation, evaluate_unary_operation}; use number::FieldElement; +use pil_analyzer::evaluator::compute_constants; pub fn optimize(mut pil_file: Analyzed) -> Analyzed { let col_count_pre = (pil_file.commitment_count(), pil_file.constant_count()); @@ -45,7 +46,7 @@ pub fn optimize_constants(mut pil_file: Analyzed) -> Analyze /// Inlines references to symbols with a single constant value. fn inline_constant_values(pil_file: &mut Analyzed) { - let constants = &pil_file.constants.clone(); + let constants = compute_constants(pil_file); pil_file.post_visit_expressions_mut(&mut |e| { if let Expression::Reference(Reference::Poly(poly)) = e { if !poly.next && poly.index.is_none() { From 764ef2db0b94d5ac8a2f4626492904c0ba8bb57e Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 4 Oct 2023 17:55:10 +0200 Subject: [PATCH 13/15] Rename function. --- pil_analyzer/src/pil_analyzer.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/pil_analyzer/src/pil_analyzer.rs b/pil_analyzer/src/pil_analyzer.rs index 39f3ddb203..0432f14836 100644 --- a/pil_analyzer/src/pil_analyzer.rs +++ b/pil_analyzer/src/pil_analyzer.rs @@ -149,7 +149,7 @@ impl PILAnalyzer { PilStatement::Include(_, include) => self.handle_include(include), PilStatement::Namespace(_, name, degree) => self.handle_namespace(name, degree), PilStatement::PolynomialDefinition(start, name, value) => { - self.handle_polynomial_definition( + self.handle_symbol_definition( self.to_source_ref(start), name, None, @@ -167,7 +167,7 @@ impl PILAnalyzer { PolynomialType::Constant, ), PilStatement::PolynomialConstantDefinition(start, name, definition) => { - self.handle_polynomial_definition( + self.handle_symbol_definition( self.to_source_ref(start), name, None, @@ -184,7 +184,7 @@ impl PILAnalyzer { PilStatement::PolynomialCommitDeclaration(start, mut polynomials, Some(definition)) => { assert!(polynomials.len() == 1); let name = polynomials.pop().unwrap(); - self.handle_polynomial_definition( + self.handle_symbol_definition( self.to_source_ref(start), name.name, name.array_size, @@ -197,7 +197,7 @@ impl PILAnalyzer { if let Err(err) = self.evaluate_expression(value.clone()) { panic!("Could not evaluate constant: {name} = {value}: {err}"); } - self.handle_polynomial_definition( + self.handle_symbol_definition( self.to_source_ref(start), name, None, @@ -238,7 +238,7 @@ impl PILAnalyzer { match value { None => { // No value provided => treat it as a witness column. - self.handle_polynomial_definition( + self.handle_symbol_definition( self.to_source_ref(start), name, None, @@ -253,7 +253,7 @@ impl PILAnalyzer { body, }) if params.len() == 1 => { // Assigned value is a lambda expression with a single parameter => treat it as a fixed column. - self.handle_polynomial_definition( + self.handle_symbol_definition( self.to_source_ref(start), name, None, @@ -269,7 +269,7 @@ impl PILAnalyzer { // Otherwise, treat it as "generic definition" SymbolKind::Other() }; - self.handle_polynomial_definition( + self.handle_symbol_definition( self.to_source_ref(start), name, None, @@ -354,7 +354,7 @@ impl PILAnalyzer { polynomial_type: PolynomialType, ) { for PolynomialName { name, array_size } in polynomials { - self.handle_polynomial_definition( + self.handle_symbol_definition( source.clone(), name, array_size, @@ -364,7 +364,7 @@ impl PILAnalyzer { } } - fn handle_polynomial_definition( + fn handle_symbol_definition( &mut self, source: SourceRef, name: String, From 61e2894206caa2746ad70adcd6d958602be4aa4b Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 10 Oct 2023 12:03:50 +0200 Subject: [PATCH 14/15] Split expression visitor for Analyzed. --- ast/src/analyzed/mod.rs | 34 ++++++++++++++++++++-- ast/src/analyzed/visitor.rs | 48 -------------------------------- pil_analyzer/src/pil_analyzer.rs | 7 +++-- pilopt/src/lib.rs | 27 ++++++++++++------ 4 files changed, 55 insertions(+), 61 deletions(-) diff --git a/ast/src/analyzed/mod.rs b/ast/src/analyzed/mod.rs index 0b4e37c0e9..d79edb9142 100644 --- a/ast/src/analyzed/mod.rs +++ b/ast/src/analyzed/mod.rs @@ -163,14 +163,16 @@ impl Analyzed { poly.id = replacements[&poly_id].id; } }); - self.pre_visit_expressions_mut(&mut |expr| { + let visitor = &mut |expr: &mut Expression<_>| { if let Expression::Reference(Reference::Poly(poly)) = expr { poly.poly_id = poly.poly_id.map(|poly_id| { assert!(!to_remove.contains(&poly_id)); replacements[&poly_id] }); } - }); + }; + self.post_visit_expressions_in_definitions_mut(visitor); + self.post_visit_expressions_in_identities_mut(visitor); } /// Adds a polynomial identity and returns the ID. @@ -225,6 +227,34 @@ impl Analyzed { retain }) } + + pub fn post_visit_expressions_in_identities_mut(&mut self, f: &mut F) + where + F: FnMut(&mut Expression), + { + self.identities + .iter_mut() + .for_each(|i| i.post_visit_expressions_mut(f)); + } + + pub fn post_visit_expressions_in_definitions_mut(&mut self, f: &mut F) + where + F: FnMut(&mut Expression), + { + // TODO add public inputs if we change them to expressions at some point. + self.definitions + .values_mut() + .for_each(|(_poly, definition)| match definition { + Some(FunctionValueDefinition::Mapping(e)) + | Some(FunctionValueDefinition::Query(e)) => e.post_visit_expressions_mut(f), + Some(FunctionValueDefinition::Array(elements)) => elements + .iter_mut() + .flat_map(|e| e.pattern.iter_mut()) + .for_each(|e| e.post_visit_expressions_mut(f)), + Some(FunctionValueDefinition::Expression(e)) => e.post_visit_expressions_mut(f), + None => {} + }); + } } #[derive(Debug, Clone)] diff --git a/ast/src/analyzed/visitor.rs b/ast/src/analyzed/visitor.rs index 11ea2d3d3a..6ebffe02c3 100644 --- a/ast/src/analyzed/visitor.rs +++ b/ast/src/analyzed/visitor.rs @@ -2,54 +2,6 @@ use crate::parsed::visitor::VisitOrder; use super::*; -impl ExpressionVisitable> for Analyzed { - fn visit_expressions_mut(&mut self, f: &mut F, o: VisitOrder) -> ControlFlow - where - F: FnMut(&mut parsed::Expression) -> ControlFlow, - { - // TODO add constants if we change them to expressions at some point. - self.definitions - .values_mut() - .try_for_each(|(_poly, definition)| match definition { - Some(FunctionValueDefinition::Mapping(e)) - | Some(FunctionValueDefinition::Query(e)) => e.visit_expressions_mut(f, o), - Some(FunctionValueDefinition::Array(elements)) => elements - .iter_mut() - .flat_map(|e| e.pattern.iter_mut()) - .try_for_each(|e| e.visit_expressions_mut(f, o)), - Some(FunctionValueDefinition::Expression(e)) => e.visit_expressions_mut(f, o), - None => ControlFlow::Continue(()), - })?; - - self.identities - .iter_mut() - .try_for_each(|i| i.visit_expressions_mut(f, o)) - } - - fn visit_expressions(&self, f: &mut F, o: VisitOrder) -> ControlFlow - where - F: FnMut(&parsed::Expression) -> ControlFlow, - { - // TODO add constants if we change them to expressions at some point. - self.definitions - .values() - .try_for_each(|(_poly, definition)| match definition { - Some(FunctionValueDefinition::Mapping(e)) - | Some(FunctionValueDefinition::Query(e)) => e.visit_expressions(f, o), - Some(FunctionValueDefinition::Array(elements)) => elements - .iter() - .flat_map(|e| e.pattern.iter()) - .try_for_each(|e| e.visit_expressions(f, o)), - Some(FunctionValueDefinition::Expression(e)) => e.visit_expressions(f, o), - None => ControlFlow::Continue(()), - })?; - - self.identities - .iter() - .try_for_each(|i| i.visit_expressions(f, o)) - } -} - impl> ExpressionVisitable for Identity { fn visit_expressions_mut(&mut self, f: &mut F, o: VisitOrder) -> ControlFlow where diff --git a/pil_analyzer/src/pil_analyzer.rs b/pil_analyzer/src/pil_analyzer.rs index 0432f14836..11f84f1805 100644 --- a/pil_analyzer/src/pil_analyzer.rs +++ b/pil_analyzer/src/pil_analyzer.rs @@ -77,11 +77,14 @@ impl From> for Analyzed { reference.poly_id = Some(poly.into()); } }; - result.pre_visit_expressions_mut(&mut |e| { + let expr_visitor = &mut |e: &mut Expression<_>| { if let Expression::Reference(Reference::Poly(reference)) = e { assign_id(reference); } - }); + }; + result.post_visit_expressions_in_definitions_mut(expr_visitor); + result.post_visit_expressions_in_identities_mut(expr_visitor); + // TODO at some point, merge public declarations with definitions as well. result .public_declarations .values_mut() diff --git a/pilopt/src/lib.rs b/pilopt/src/lib.rs index b99acb82ff..553e141e1c 100644 --- a/pilopt/src/lib.rs +++ b/pilopt/src/lib.rs @@ -47,7 +47,7 @@ pub fn optimize_constants(mut pil_file: Analyzed) -> Analyze /// Inlines references to symbols with a single constant value. fn inline_constant_values(pil_file: &mut Analyzed) { let constants = compute_constants(pil_file); - pil_file.post_visit_expressions_mut(&mut |e| { + let visitor = &mut |e: &mut Expression<_>| { if let Expression::Reference(Reference::Poly(poly)) = e { if !poly.next && poly.index.is_none() { if let Some(value) = constants.get(&poly.name) { @@ -55,12 +55,14 @@ fn inline_constant_values(pil_file: &mut Analyzed) { } } } - }); + }; + pil_file.post_visit_expressions_in_definitions_mut(visitor); + pil_file.post_visit_expressions_in_identities_mut(visitor); } /// Substitutes expression that evaluate to a constant value. fn evaluate_constant_subtrees(pil_file: &mut Analyzed) { - pil_file.post_visit_expressions_mut(&mut |e| match e { + let visitor = &mut |e: &mut Expression<_>| match e { Expression::BinaryOperation(left, op, right) => { if let (Expression::Number(l), Expression::Number(r)) = (left.as_ref(), right.as_ref()) { @@ -73,7 +75,9 @@ fn evaluate_constant_subtrees(pil_file: &mut Analyzed) { } } _ => {} - }); + }; + pil_file.post_visit_expressions_in_definitions_mut(visitor); + pil_file.post_visit_expressions_in_identities_mut(visitor); } /// Identifies fixed columns that only have a single value, replaces every @@ -95,7 +99,7 @@ fn remove_constant_fixed_columns(pil_file: &mut Analyzed) { }) .collect::>(); - pil_file.pre_visit_expressions_mut(&mut |e| { + let visitor = &mut |e: &mut Expression<_>| { if let Expression::Reference(Reference::Poly(PolynomialReference { name: _, index, @@ -108,7 +112,9 @@ fn remove_constant_fixed_columns(pil_file: &mut Analyzed) { *e = Expression::Number(*value); } } - }); + }; + pil_file.post_visit_expressions_in_definitions_mut(visitor); + pil_file.post_visit_expressions_in_identities_mut(visitor); pil_file.remove_polynomials(&constant_polys.keys().cloned().collect()); } @@ -143,7 +149,8 @@ fn constant_value(function: &FunctionValueDefinition) -> Opt /// Simplifies multiplications by zero and one. fn simplify_expressions(pil_file: &mut Analyzed) { - pil_file.post_visit_expressions_mut(&mut simplify_expression_single); + pil_file.post_visit_expressions_in_definitions_mut(&mut simplify_expression_single); + pil_file.post_visit_expressions_in_identities_mut(&mut simplify_expression_single); } fn simplify_expression(mut e: Expression) -> Expression { @@ -315,7 +322,7 @@ fn remove_constant_witness_columns(pil_file: &mut Analyzed) .filter_map(|expr| constrained_to_constant(expr)) .collect::>(); - pil_file.pre_visit_expressions_mut(&mut |e| { + let visitor = &mut |e: &mut Expression<_>| { if let Expression::Reference(Reference::Poly(PolynomialReference { name: _, index, @@ -328,7 +335,9 @@ fn remove_constant_witness_columns(pil_file: &mut Analyzed) *e = Expression::Number(*value); } } - }); + }; + pil_file.post_visit_expressions_in_definitions_mut(visitor); + pil_file.post_visit_expressions_in_identities_mut(visitor); pil_file.remove_polynomials(&constant_polys.keys().cloned().collect()); } From 1dbae8849caaec255dc41b36e93f69c0af181191 Mon Sep 17 00:00:00 2001 From: Leo Date: Mon, 9 Oct 2023 16:52:55 +0200 Subject: [PATCH 15/15] Add .circleci/config.yml --- .circleci/config.yml | 105 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 .circleci/config.yml diff --git a/.circleci/config.yml b/.circleci/config.yml new file mode 100644 index 0000000000..da0f5bd0c5 --- /dev/null +++ b/.circleci/config.yml @@ -0,0 +1,105 @@ +version: 2.1 + +jobs: + pr-tests: + machine: + image: ubuntu-2004:current + resource_class: large + steps: + - run: + name: Install dependencies + command: sudo apt update -qqy && sudo apt install libclang-dev git nodejs -qqy + + - run: + name: Install Rust + command: curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y + + - checkout + + - run: + name: Install pilcom + command: git clone https://github.com/0xPolygonHermez/pilcom.git + + - restore_cache: + name: Restore pilcom modules cache + keys: + - v5-pilcom-node-modules-{{ checksum "pilcom/package-lock.json" }} + + - run: + name: Install pilcom node modules if cache miss + command: | + if [ ! -d "pilcom/node_modules" ]; then + (cd pilcom && npm install) + fi + + - save_cache: + name: Save pilcom modules cache + key: v5-pilcom-node-modules-{{ checksum "pilcom/package-lock.json" }} + paths: + - pilcom/node_modules + + - restore_cache: + name: Restore Rust cache + keys: + - v5-cargo-pr-tests-{{ checksum "Cargo.toml" }} + + - run: + name: Install Rust toolchain 1.72 (with clippy and rustfmt) + command: | + rustup toolchain install 1.72-x86_64-unknown-linux-gnu + rustup component add clippy --toolchain 1.72-x86_64-unknown-linux-gnu + rustup component add rustfmt --toolchain 1.72-x86_64-unknown-linux-gnu + + - run: + name: Install nightly + command: rustup toolchain install nightly-2023-01-03-x86_64-unknown-linux-gnu + + - run: + name: Install riscv target + command: rustup target add riscv32imac-unknown-none-elf --toolchain nightly-2023-01-03-x86_64-unknown-linux-gnu + + - run: + name: Install stdlib + command: rustup component add rust-src --toolchain nightly-2023-01-03-x86_64-unknown-linux-gnu + + - run: + name: Check without Halo2 + command: cargo check --all --no-default-features --profile pr-tests + + - run: + name: Build + command: cargo build --all --all-features --profile pr-tests + + - save_cache: + name: Save Rust cache + key: v5-cargo-pr-tests-{{ checksum "Cargo.toml" }} + paths: + - ~/.cargo/registry + - ~/.cargo/git + - target + + - run: + name: Run default tests + command: PILCOM=$(pwd)/pilcom/ cargo test --all --all-features --profile pr-tests --verbose + + - run: + name: Run slow tests + command: PILCOM=$(pwd)/pilcom/ cargo test --all --all-features --profile pr-tests --verbose -- --ignored --nocapture --test-threads=1 --exact test_keccak test_vec_median instruction_tests::addi + + - run: + name: Lint + command: cargo clippy --all --all-features -- -D warnings + + - run: + name: Format + command: cargo fmt --all --check --verbose + + - run: + name: Check benches compile without running them + command: cargo bench --all --all-features --profile pr-tests --no-run + +workflows: + version: 2 + build-and-test: + jobs: + - pr-tests