0xPolygonZero / plonky2

Apache License 2.0
758 stars 281 forks source link

Replace increment macro with an opcode. #1412

Closed LindaGuiga closed 8 months ago

LindaGuiga commented 9 months ago

This PR replaces all %increment (and %add_const(1)) calls by an INCREMENT custom opcode. This saves 1435 cycles on the CPU side (and 7356 cycles on memory side) for erc20, but this comes at the expense of one additional ArithmeticStark column.

sonarcloud[bot] commented 9 months ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Nashtare commented 9 months ago

I hadn't realized the gains would be that light (makes sense, it's halving the overhead of all increments), hence we may want to consider all possibilities around custom instructions before inserting new ones, especially if it incurs additional columns elsewhere?

LindaGuiga commented 9 months ago

I hadn't realized the gains would be that light (makes sense, it's halving the overhead of all increments), hence we may want to consider all possibilities around custom instructions before inserting new ones, especially if it incurs additional columns elsewhere?

It was made even lighter than expected because of the new pre-loading optimizations I think (originally, the gains were supposed to be higher than that). I agree it now makes sense to look into all possible instructions.

Nashtare commented 8 months ago

@LindaGuiga Should we close this?

LindaGuiga commented 8 months ago

I think it makes sense to close it yes. I don't think this is worth it as is.