printOperation(tensor::InsertOp) at
lib/Target/OpenFhePke/OpenFhePkeEmitter.cpp:1708-1729 unconditionally
mutates the destination in place and calls
variableNames->mapValueNameToValue(op.getResult(), op.getDest()),
aliasing the SSA result's C++ variable name to the destination buffer.
This alias is unsafe when the destination has more than one use, for
example when two sequential tensor.insert %ct into %buf[c0] operations
both consume the same buffer and produce two SSA values that are
independently consumed downstream. Both SSA values then print as the same
C++ variable, and any consumer of the first one silently gets the mutated
(second) value.
The sibling printOperation(tensor::InsertSliceOp) at lines 1565-1581
already does the correct thing: it allocates a fresh std::vector<T> vResult(vDest);
when the result and dest names differ, then writes into
the copy. The scalar InsertOp should follow the same pattern.
We hit this in a multi-result kernel where the --scheme-to-openfhe
conversion wraps each scalar !ct result into a tensor<1x!ct> via
tensor.insert, and both wraps share the same tensor.empty buffer.
The emitted C++ becomes:
std::vector<CiphertextT> v51(1);
v51[0] = ct133; // %wrap1 aliased to v51
v51[0] = ct142; // %wrap2 also aliased to v51
return {v51, v51}; // both fields = last-written v51
Combined with the shared_ptr semantics of Ciphertext<DCRTPoly>, both
multi-result fields carry the same ciphertext at runtime.
Suggested fix (drop-in from the InsertSliceOp pattern): in
printOperation(tensor::InsertOp), when getNameForValue(op.getResult()) != getNameForValue(op.getDest()), emit
std::vector<T> vResult(vDest); vResult[idx] = scalar; and do NOT call
mapValueNameToValue.
Minimal reproducer (also usable as a regression test):
// RUN: heir-translate --emit-openfhe-pke %s | FileCheck %s
!cc = !openfhe.crypto_context
!ct = !openfhe.ciphertext
// CHECK-LABEL: mm_bug_two_wraps
// CHECK: std::vector<{{.*}}> [[V1:v[0-9]+]](
// CHECK: [[V1]][0] = %arg1
// CHECK: std::vector<{{.*}}> [[V2:v[0-9]+]](
// CHECK: [[V2]][0] = %arg2
// CHECK: return {[[V1]], [[V2]]};
func.func @mm_bug_two_wraps(%arg0: !ct, %arg1: !ct, %arg2: !ct)
-> (tensor<1x!ct>, tensor<1x!ct>) {
%c0 = arith.constant 0 : index
%buf = tensor.empty() : tensor<1x!ct>
%wrap1 = tensor.insert %arg1 into %buf[%c0] : tensor<1x!ct>
%wrap2 = tensor.insert %arg2 into %buf[%c0] : tensor<1x!ct>
return %wrap1, %wrap2 : tensor<1x!ct>, tensor<1x!ct>
}
Likely also affects the Lattigo emitter (analogous
printOperation(tensor::InsertOp) at lib/Target/Lattigo/) — should be
checked and patched symmetrically.
Affected files:
- lib/Target/OpenFhePke/OpenFhePkeEmitter.cpp:1708-1729 (root cause)
PS: Triaged using claude opus
printOperation(tensor::InsertOp)atlib/Target/OpenFhePke/OpenFhePkeEmitter.cpp:1708-1729unconditionallymutates the destination in place and calls
variableNames->mapValueNameToValue(op.getResult(), op.getDest()),aliasing the SSA result's C++ variable name to the destination buffer.
This alias is unsafe when the destination has more than one use, for
example when two sequential
tensor.insert %ct into %buf[c0]operationsboth consume the same buffer and produce two SSA values that are
independently consumed downstream. Both SSA values then print as the same
C++ variable, and any consumer of the first one silently gets the mutated
(second) value.
The sibling
printOperation(tensor::InsertSliceOp)at lines 1565-1581already does the correct thing: it allocates a fresh
std::vector<T> vResult(vDest);when the result and dest names differ, then writes into
the copy. The scalar
InsertOpshould follow the same pattern.We hit this in a multi-result kernel where the
--scheme-to-openfheconversion wraps each scalar
!ctresult into atensor<1x!ct>viatensor.insert, and both wraps share the sametensor.emptybuffer.The emitted C++ becomes:
Combined with the shared_ptr semantics of
Ciphertext<DCRTPoly>, bothmulti-result fields carry the same ciphertext at runtime.
Suggested fix (drop-in from the InsertSliceOp pattern): in
printOperation(tensor::InsertOp), whengetNameForValue(op.getResult()) != getNameForValue(op.getDest()), emitstd::vector<T> vResult(vDest); vResult[idx] = scalar;and do NOT callmapValueNameToValue.Minimal reproducer (also usable as a regression test):
Likely also affects the Lattigo emitter (analogous
printOperation(tensor::InsertOp)atlib/Target/Lattigo/) — should bechecked and patched symmetrically.
Affected files:
PS: Triaged using claude opus