Skip to content

openfhe-alloc-to-inplace overwrites value which has uses. #3195

Description

@VedantParanjape

The --openfhe-alloc-to-inplace (and --lattigo-alloc-to-inplace) passes
promote openfhe.add / lattigo.add to their in-place variants using a
liveness check that only queries MLIR's Liveness::isDeadAfter
(lib/Utils/AllocToInPlaceUtils.h:355-360, isSafeToMutateInPlace). This
misses the case where the first operand is captured by a func.return,
secret.yield, or tensor.insert{,_slice} op earlier in program order:
MLIR liveness reports the operand as dead after the add because its next
use is the mutation itself, but the outlasting capture still needs the
pre-mutation value. Since Ciphertext<DCRTPoly> (OpenFHE) and its Lattigo
analogue are reference-counted / shared-pointer types in the generated C++,
the in-place mutation corrupts the value observed by every other consumer
of the same SSA def.

We hit this in a multi-ciphertext output kernel where two
secret.yield results (%sum and %sum + %rot) end up both equal to
%sum + %rot at runtime, silently producing "decrypted output disagrees
with reference" errors — symbolic verification passes because SSA semantics
model each value as immutable.

Suggested fix: extend isSafeToMutateInPlace to reuse the existing
CallerProvidedStorageInfo::isStored helper
(AllocToInPlaceUtils.h:59-93), which already knows how to detect capture
by tensor.insert{,_slice} / scf.yield / func.return, and reject
promotion when any such user exists downstream of the mutation site.

Minimal reproducer (also usable as a regression test — the CHECK-NOT
directives fail on current HEAD):

// RUN: heir-opt --openfhe-alloc-to-inplace %s | FileCheck %s
!cc = !openfhe.crypto_context
!ct = !openfhe.ciphertext
// CHECK-LABEL: func.func @bug_returned_operand_still_live
func.func @bug_returned_operand_still_live(
    %cc: !cc, %ct_a: !ct, %ct_b: !ct, %ct_c: !ct) -> (!ct, !ct) {
  %sum  = openfhe.add %cc, %ct_a, %ct_b : (!cc, !ct, !ct) -> !ct
  // CHECK-NOT: openfhe.add_inplace %{{.*}}, %sum
  %sum2 = openfhe.add %cc, %sum,  %ct_c : (!cc, !ct, !ct) -> !ct
  return %sum, %sum2 : !ct, !ct
}

Affected files:

  • lib/Dialect/Openfhe/Transforms/AllocToInPlace.cpp:33-53
  • lib/Utils/AllocToInPlaceUtils.h:355-360 (root cause)
  • lib/Dialect/Lattigo/Transforms/AllocToInPlace.cpp (same helper, same bug)
// Reproducer: `openfhe.add` is promoted to `openfhe.add_inplace` even when
// the first operand is still live via a `func.return` (or a
// `tensor.insert{,_slice}` / `secret.yield` that outlasts the mutation site).
// The lattigo `--lattigo-alloc-to-inplace` and openfhe
// `--openfhe-alloc-to-inplace` passes share the buggy liveness check in
// `lib/Utils/AllocToInPlaceUtils.h:355-360` (`isSafeToMutateInPlace`) — it
// only queries MLIR `Liveness::isDeadAfter`, which treats the second use of
// the operand as "dead after" the first use in program order even if that
// first use is a mutation of the same shared storage.
//
// Impact: `Ciphertext<DCRTPoly>` (OpenFHE) is a `std::shared_ptr` in the
// C++ runtime. `EvalAddInPlace(a, b)` mutates the pointed-to object. If any
// other Op captured the shared_ptr (return, tensor.insert, secret.yield),
// that consumer sees the mutated value instead of the pre-add snapshot.
//
// RUN: heir-opt --openfhe-alloc-to-inplace %s | FileCheck %s

!cc = !openfhe.crypto_context
!ct = !openfhe.ciphertext

// Minimal shape: compute %sum, then compute %sum2 = %sum + %ct_c, return
// both. Symbolic (SSA) semantics: result 0 = %sum, result 1 = %sum + %ct_c.
// Buggy runtime (after add_inplace promotion): result 0 == result 1.
//
// CHECK-LABEL: func.func @bug_returned_operand_still_live
func.func @bug_returned_operand_still_live(
    %cc: !cc, %ct_a: !ct, %ct_b: !ct, %ct_c: !ct) -> (!ct, !ct) {
  %sum = openfhe.add %cc, %ct_a, %ct_b : (!cc, !ct, !ct) -> !ct

  // BUG: the pass promotes this to `openfhe.add_inplace %cc, %sum, %ct_c`,
  // mutating %sum. But %sum is captured by the func.return below — its
  // pre-mutation value is required, so the promotion is unsafe.
  //
  // A correct pass output would keep this as `openfhe.add` (out-of-place),
  // so %sum stays intact for the return.
  //
  // CHECK-NOT: openfhe.add_inplace %{{.*}}, %sum
  %sum2 = openfhe.add %cc, %sum, %ct_c : (!cc, !ct, !ct) -> !ct

  return %sum, %sum2 : !ct, !ct
}

// Same bug shape but with %sum captured by a `tensor.insert` first (as
// produced by HEIR's multi-ct output-packing lowering) rather than
// directly by a return. Same root cause: MLIR liveness treats %sum as dead
// after the `openfhe.add` because its next use is the mutation itself, but
// the `%packed` tensor built from %sum before the add still needs %sum's
// pre-mutation value.
//
// CHECK-LABEL: func.func @bug_captured_by_tensor_insert
func.func @bug_captured_by_tensor_insert(
    %cc: !cc, %ct_a: !ct, %ct_b: !ct, %ct_c: !ct) -> (tensor<2x!ct>) {
  %c0 = arith.constant 0 : index
  %c1 = arith.constant 1 : index
  %sum = openfhe.add %cc, %ct_a, %ct_b : (!cc, !ct, !ct) -> !ct
  %empty = tensor.empty() : tensor<2x!ct>
  %with_sum = tensor.insert %sum into %empty[%c0] : tensor<2x!ct>

  // BUG: promoted to add_inplace, mutating %sum. The `%with_sum` tensor's
  // slot 0 shares the shared_ptr and now sees the mutated value.
  //
  // CHECK-NOT: openfhe.add_inplace %{{.*}}, %sum
  %sum2 = openfhe.add %cc, %sum, %ct_c : (!cc, !ct, !ct) -> !ct
  %packed = tensor.insert %sum2 into %with_sum[%c1] : tensor<2x!ct>
  return %packed : tensor<2x!ct>
}

PS: Triaged using claude opus

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions