google / google/heir

Migrate dialect conversion passes to `allowPatternRollback = false`

Open
#2,210 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
MLIR
Stars
906
Forks
171
Avg merge
4d 12h
Merged PRs (30d)
32

Description

Most of them migrate smoothly. The following fail

- lib/Dialect/Arith/Conversions/ArithToCGGI/ArithToCGGI.cpp
- lib/Transforms/MemrefToArith/MemrefGlobalReplace.cpp

MemrefGlobalReplace will be deleted I think, so it can stay opted out.

ArithToCGGI has tests that... don't run secret-wrap-generic at the start? (e.g., tests/Examples/tfhe_rust_hl/cpu/fully_connected.mlir starts with --mlir-to-cggi so the {secret.secret} annotation is never touched). That test crashes at materialization because it's trying to materialize an unrealized conversion cast of a memref function argument, but with allowPatternRollback=true those materializations are never needed or run in the first place. So I suspect this pass could be fixed by instead properly converting the input IR to use secret.generic, secret types, and split-generic before converting to CGGI.

IR dump after failure:

```
// -----// IR Dump After CGGIToTfheRust Failed (cggi-to-tfhe-rust) //----- //
#constant_coefficient_encoding = #lwe.constant_coefficient_encoding
#constant_coefficient_encoding1 = #lwe.constant_coefficient_encoding
#key = #lwe.key<>
#ring_i32_1 = #polynomial.ring>
#ring_i8_1 = #polynomial.ring>
#ciphertext_space__D742 = #lwe.ciphertext_space
!ct__D742 = !lwe.lwe_ciphertext>, plaintext_space = , ciphertext_space = #ciphertext_space__D742, key = #key>
!ct__D742_1 = !lwe.lwe_ciphertext>, plaintext_space = , ciphertext_space = #ciphertext_space__D742, key = #key>
module {
func.func @fn_under_test(%arg0: memref<1x1x!tfhe_rust.eui8> {secret.secret}) -> memref<1x1x!tfhe_rust.eui32> {
%0 = builtin.unrealized_conversion_cast %arg0 : memref<1x1x!tfhe_rust.eui8> to memref<1x1x!ct__D742, strided<[?, ?], offset: ?>>
%c0 = arith.constant 0 : index
%c2_i16 = arith.constant 2 : i16
%c1_i16 = arith.constant 1 : i16
%1 = builtin.unrealized_conversion_cast %0 : memref<1x1x!ct__D742, strided<[?, ?], offset: ?>> to memref<1x1xi8, strided<[?, ?], offset: ?>>
%2 = builtin.unrealized_conversion_cast %1 : memref<1x1xi8, strided<[?, ?], offset: ?>> to memref<1x1xi8>
%alloc = memref.alloc() {alignment = 64 : i64} : memref<1x1xi32>
%3 = builtin.unrealized_conversion_cast %alloc : memref<1x1xi32> to memref<1x1x!ct__D742_1>
%4 = builtin.unrealized_conversion_cast %3 : memref<1x1x!ct__D742_1> to memref<1x1x!tfhe_rust.eui32>
%5 = memref.load %2[%c0, %c0] : memref<1x1xi8>
%6 = arith.extsi %5 : i8 to i16
%7 = arith.muli %6, %c2_i16 : i16
%8 = arith.addi %7, %c1_i16 : i16
%9 = arith.extsi %8 : i16 to i32
memref.store %9, %alloc[%c0, %c0] : memref<1x1xi32>
return %4 : memref<1x1x!tfhe_rust.eui32>
}
}
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.