On Tue, Aug 6, 2024, at 13:52, Dean Rasheed wrote: > On Mon, 5 Aug 2024 at 13:34, Joel Jacobson <j...@compiler.org> wrote: >> >> Noted from 5e1f3b9eb: >> "While it adds some space on 32-bit machines, we aren't optimizing for that >> case anymore." >> >> In this case, the extra 32-bit numeric_mul code seems to be 89 lines of >> code, excluding comments. >> To me, this seems like quite a lot, so I lean on thinking we should omit >> that code for now. >> We can always add it later if we get pushback. >> > > OK, I guess that's reasonable. There is no clear-cut right answer > here, but I don't really want to have a lot of 32-bit-specific code > that significantly complicates this function, making it harder to > maintain. Without that code, the patch becomes much simpler, which > seems like a decent justification for any performance tradeoffs on > 32-bit machines that are unlikely to affect many people anyway. > > Regards, > Dean > > Attachments: > * v4-0001-Extend-mul_var_short-to-5-and-6-digit-inputs.patch > * v4-0002-Optimise-numeric-multiplication-using-base-NBASE-.patch
I've reviewed and tested both patches and think they are ready to be committed. Neat with the pairs variables, really improved readability a lot, compared to my first version. Also neat you found a way to adjust the res_weight in a simpler way than my quite lengthy expression. Regards, Joel