Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Epic: Expand code gen attributes to support inlining after flattening #4877

Closed
3 tasks done
vezenovm opened this issue Apr 22, 2024 · 1 comment
Closed
3 tasks done
Assignees

Comments

@vezenovm
Copy link
Contributor

vezenovm commented Apr 22, 2024

Problem

After #4428 we added support for non-inlining ACIR functions. However, this only included a Fold inline type where we indicate to a backend that the ACIR circuit is an entry point to be folded with the other ACIR circuits that make up a program. This is more of a ZK circuit specific attribute that happens to also be a code gen attribute (as we previously inlined all ACIR functions into a single circuit).

However, in certain cases, such as heavy array logic dependent upon witness conditions (#4688), it may be more efficient to generate code in a different way than the default of inlining every ACIR function call (see #4834 for the results of folding the poseidon call in #4688). Fold is insufficient for every use-case of non-inlining though. A user may still desire proving a single circuit, but also providing well-placed inline hints on hot functions may allow them to optimize code generation and the resulting code itself.

Happy Case

We should allow users to specify #[no_predicates] on hot functions for which they do not wish to repeatedly generate the same code.

Ideally the code from issue #4688 should be able to be optimized as such:

use dep::std::hash::{pedersen_hash_with_separator, poseidon2::Poseidon2};

global NUM_HASHES = 2;
global HASH_LENGTH = 10;

#[inline(never)]
pub fn poseidon_hash<N>(inputs: [Field; N]) -> Field {
    Poseidon2::hash(inputs, inputs.len())
}
fn main(
    to_hash: [[Field; HASH_LENGTH]; NUM_HASHES],
    enable: [bool; NUM_HASHES]
) -> pub [Field; NUM_HASHES] {
    let mut result = [0; NUM_HASHES];
    for i in 0..NUM_HASHES {
        let enable = enable[i];
        let to_hash = to_hash[i];
        if enable {
            result[i] = poseidon_hash(to_hash);
        }
    }
    result
}

A user could then generate a single circuit for which we can prove/verify as expected for a vanilla circuit.

The same code written with a black box function such as pedersen_hash will give us about 10 opcodes for main. This is because when working with the black box function we do not inline the black box logic and flattening of the if statement and handling of its predicate is done only on the result of the black box call. By allowing the developer to specify #[inline(never)] we can enable this same optimization on regular ACIR functions.

Tasks

  1. compiler enhancement
  2. ACIR/ACVM enhancement ssa
  3. enhancement

Project Impact

Blocker

Impact Context

We had tried other optimizations for #4688 (such as #4716), but they are non-trivial. According to the results from #4834 we can achieve real improvements in our code generation by non-inlining certain hot functions.

Workaround

None

Workaround Description

No response

Additional Context

No response

Would you like to submit a PR for this Issue?

None

Support Needs

No response

@vezenovm vezenovm changed the title Code gen: Expand attributes to support #[inline(never)] Epic: Expand code gen attributes to support #[inline(never)] Apr 24, 2024
@vezenovm vezenovm changed the title Epic: Expand code gen attributes to support #[inline(never)] Epic: Expand code gen attributes to support #[inline(tag)] Apr 24, 2024
@vezenovm vezenovm self-assigned this Apr 26, 2024
@vezenovm vezenovm changed the title Epic: Expand code gen attributes to support #[inline(tag)] Epic: Expand code gen attributes to support inlining after flattening Apr 30, 2024
@vezenovm
Copy link
Contributor Author

vezenovm commented May 1, 2024

Closing after #4911

@vezenovm vezenovm closed this as completed May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

1 participant