Remove old layout code#2199
Merged
copybara-service[bot] merged 1 commit intogoogle:mainfrom Oct 10, 2025
Merged
Conversation
44ba3d9 to
6e983c4
Compare
4fbdb96 to
020545f
Compare
6c34244 to
3c6d4a2
Compare
a6d2649 to
1eeb9ea
Compare
j2kun
commented
Oct 7, 2025
7bc3d56 to
9dd6e1a
Compare
1c874f3 to
e674615
Compare
j2kun
referenced
this pull request
in google/github_workflows_playground
Oct 8, 2025
asraa
approved these changes
Oct 9, 2025
Collaborator
asraa
left a comment
There was a problem hiding this comment.
phew! appreciate so much how much the PR description helped here
tests/Dialect/Secret/Conversions/secret_to_bgv/ciphertext_plaintext.mlir
Show resolved
Hide resolved
lib/Dialect/Secret/Conversions/SecretToModArith/SecretToModArith.cpp
Outdated
Show resolved
Hide resolved
This PR removes the old layout code (based on plain affine maps and alignment attributes) and migrates all existing code paths to the new layout system based on integer relations. As a byproduct, this commit adds support for multi-packed ciphertexts through the entire arithmetic pipeline of HEIR. Co-authored-by: asraa <asraa@google.com>
15bbdfe to
49b6f20
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR removes the old layout code (based on plain affine maps and alignment attributes) and migrates all existing code paths to the new layout system based on integer relations.
I will attempt to summarize the many changes that went into this. The short version is: I had to clean up a lot of technical debt from our prior lack of support for multi-packed ciphertexts, and this impacted pretty much every part of the arithmetic pipelines.
Pipeline changes
elementwise-to-affinestep that converts batch scheme ops to loops.implement-shift-networkhas been added to themlir-to-secret-arithmeticsubpipelineimplement-rotate-and-reducebaby-step giant-step optimization, this can produce rotations of server-side constant plaintexts, and those are not lowered in secret-to-scheme because they have no secret operands or results. So the arithmetic pipelines now have a step oftensor-ext-to-tensorafter all secret tensor-ext ops are lowered.FoldRotateOfConst, was added to fold a rotated dense constant (e.g., a plaintext mask) to the corresponding pre-rotated dense constant.fold-constant-tensorswas added to help simplify extract_slice of tensor.splat which occurred as an edge case of thecmuxtest; the then/else branches were dynamically-valued cleartext scalars but the test was a ciphertext, so the then/else values got layouts implemented as a splat, and then were immediately slice-extracted for plaintext-ciphertext ops. So this pattern folds that to another splat and removes the original splat.Pass changes
General
getElementTypeOrSelfto replace it.{numSlots}and this was changed to check the trailing axis.addTensorConversionPatternswas added to generically type-convert operands and results of "blind" tensor ops in lowerings likelwe-to-lattigo.llvm_runnertests were usingStridedMemRefType<int64_t>for the C API, which defaults to a rank-1 tensor, and so for multi-packed ciphertexts these were updated toStridedMemRefType<int64_t, 2>or similar.secret-to-<scheme>secret-to-schemeconversions are limited to operating on full ciphertexts (i.e., you can only extract a slice from a ciphertext-semantic tensor if you're extracting one or more full rows).secret-to-mod-arithsecret-to-mod-arithwas using context-aware conversion patterns when it should not have been.secret-to-ckksThis pass had a number of hard-coded handling of "multi-packed" ciphertexts that were added purely to make the initial halevi-shoup matvec demo work from Summer of 2024. This code is all removed and the corresponding ops are supported in generality.
tensor-ext-to-tensor(submitted separately as #2291)
layout-propagationOperandLayoutRequirementOpInterfacegot a newisSecretoption becausetensor.inserthas two regimes: if it's inserting a cleartext scalar into a plaintext or ciphertext, the scalar does not require a layout. If it's inserting a ciphertext scalar, then the scalar does require a layout.rectifyIncompatibleOperandLayoutswas added fortensor.insert, which handles the case of inserting a secret scalar into a secret destination by layout-converting the scalar to the same layout as the dest tensor. This is bad but it's also bad to insert scalars into secret tensors so I don't expect this kernel to be used all that much. (It was in a test that did a manual matvec mul by a nested loop with inner body extract, mul, sum, insert)tensor.extractop now does not require replicating the scalar to all slots. This allows us to avoid some layout conversions that are unnecessary when the tensor.extract is the last operation in the IR, and in other cases where the result is used in a way that doesn't strictly require full replication of the scalar in every slot.secret-insert-mgmtAll these passes now add
mgmt.initops for relevant tensor operations that have plaintext operands (e.g., insert_slice into a plaintexttensor.empty)Emitter Changes
Issues