Skip to content

feat(avm): instruction fetching parsing error#12804

Merged
jeanmon merged 25 commits into
masterfrom
jm/instr-fetch-parsing-error
Mar 20, 2025
Merged

feat(avm): instruction fetching parsing error#12804
jeanmon merged 25 commits into
masterfrom
jm/instr-fetch-parsing-error

Conversation

@jeanmon

@jeanmon jeanmon commented Mar 17, 2025

Copy link
Copy Markdown
Contributor

No description provided.

@jeanmon jeanmon force-pushed the jm/instr-fetch-parsing-error branch from 8e290fa to 0a2d781 Compare March 17, 2025 17:22
SumcheckRoundUnivariate compute_univariate(ProverPolynomialsOrPartiallyEvaluatedMultivariates& polynomials,
const bb::RelationParameters<FF>& relation_parameters,
const bb::GateSeparatorPolynomial<FF>& gate_sparators,
const bb::GateSeparatorPolynomial<FF>& gate_separators,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Corrected a typo as part of this PR in sumcheck_round.hpp. Not relevant for the review.

namespace bb::avm2 {

template <typename T>
requires(std::unsigned_integral<T>)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Was ported from vm1

@jeanmon jeanmon force-pushed the jm/instr-fetch-parsing-error branch from 0a2d781 to 43f24fa Compare March 17, 2025 17:33
@jeanmon jeanmon marked this pull request as ready for review March 17, 2025 17:45
Comment thread barretenberg/cpp/pil/vm2/instr_fetching.pil Outdated
Comment thread barretenberg/cpp/pil/vm2/instr_fetching.pil Outdated
@jeanmon jeanmon force-pushed the jm/instr-fetch-parsing-error branch from 81962c8 to 98f2f73 Compare March 18, 2025 12:07
@netlify

netlify Bot commented Mar 18, 2025

Copy link
Copy Markdown

Deploy Preview for barretenberg failed. Why did it fail? →

Name Link
🔨 Latest commit 98f2f73
🔍 Latest deploy log https://app.netlify.com/sites/barretenberg/deploys/67d961e6d375c400085ddfe7

@fcarreiro fcarreiro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Will review C++ but left some PIL comments already.

Comment thread barretenberg/cpp/pil/vm2/instr_fetching.pil Outdated
Comment thread barretenberg/cpp/pil/vm2/instr_fetching.pil Outdated
Comment thread barretenberg/cpp/pil/vm2/instr_fetching.pil Outdated
Comment thread barretenberg/cpp/pil/vm2/instr_fetching.pil Outdated
Comment thread barretenberg/cpp/pil/vm2/instr_fetching.pil Outdated
Comment thread barretenberg/cpp/src/barretenberg/vm2/tracegen/bytecode_trace.cpp Outdated
Comment thread barretenberg/cpp/pil/vm2/precomputed.pil
Comment thread barretenberg/cpp/pil/vm2/instr_fetching.pil Outdated
@jeanmon jeanmon marked this pull request as draft March 18, 2025 15:19
@jeanmon jeanmon force-pushed the jm/instr-fetch-parsing-error branch 3 times, most recently from ebb9686 to fab4e8e Compare March 19, 2025 12:21

/**
* @brief Scale Univaraites, each representing a subrelation, by different challenges
* @brief Scale Univariates, each representing a subrelation, by different challenges

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Just a corrected typo as part of this PR.

@fcarreiro fcarreiro self-requested a review March 19, 2025 12:35

@fcarreiro fcarreiro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looking better!

Comment thread barretenberg/cpp/src/barretenberg/vm2/simulation/lib/serialization.cpp Outdated
Comment thread barretenberg/cpp/src/barretenberg/vm2/simulation/lib/serialization.cpp Outdated
Comment thread barretenberg/cpp/src/barretenberg/vm2/simulation/lib/serialization.hpp Outdated
Comment thread barretenberg/cpp/src/barretenberg/vm2/simulation_helper.cpp
Comment thread barretenberg/cpp/src/barretenberg/vm2/simulation/bytecode_manager.cpp Outdated
Comment thread barretenberg/cpp/src/barretenberg/vm2/tracegen/precomputed_trace.cpp Outdated
Comment thread barretenberg/cpp/pil/vm2/precomputed.pil Outdated
Comment thread barretenberg/cpp/src/barretenberg/vm2/tracegen/bytecode_trace.cpp Outdated
Comment thread barretenberg/cpp/src/barretenberg/vm2/tracegen_helper.cpp

@fcarreiro fcarreiro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, but will let Ilyas check the PIL and approve. Thanks for all the changes!

Comment thread barretenberg/cpp/src/barretenberg/vm2/tracegen_helper.cpp
@jeanmon jeanmon force-pushed the jm/instr-fetch-parsing-error branch from 4553dda to ea7b570 Compare March 20, 2025 08:12
@jeanmon jeanmon marked this pull request as ready for review March 20, 2025 08:12

@IlyasRidhuan IlyasRidhuan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good stuff! Looks good to me!

@jeanmon jeanmon merged commit 09a09d5 into master Mar 20, 2025
@jeanmon jeanmon deleted the jm/instr-fetch-parsing-error branch March 20, 2025 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants