#1992: Convert to Ciphertext Semantics Update

[ai-mannamalai]

• Delcare Arith dialect as as illegal in conversion target

[asraa]

shouldn't we be expecting arithmetic operations in the output of this pass though? this pass intends to arithmetize / pack, so that we aren't working on the cleartext data types but are working on ciphertext types with rotations and SIMD operations (including arithmetic adds and muls)

Maybe we should just make all ops in the arith dialect BUT the ones supported by ciphertext semantics illegal? (e.g. index_cast would be illegal, but constants / muls / adds would be legal)

[ai-mannamalai]

If we want to do that the following mechanism in MLIR is useful:

  /// Register the operations of the given dialects as dynamically legal, i.e.
  /// requiring custom handling by the callback.
  template <typename... Names>
  void addDynamicallyLegalDialect(const DynamicLegalityCallbackFn &callback,
                                  StringRef name, Names... names) {
    SmallVector<StringRef, 2> dialectNames({name, names...});
    setDialectAction(dialectNames, LegalizationAction::Dynamic);
    setLegalityCallback(dialectNames, callback);
  }
  template <typename... Args>
  void addDynamicallyLegalDialect(DynamicLegalityCallbackFn callback) {
    addDynamicallyLegalDialect(std::move(callback),
                               Args::getDialectNamespace()...);
  }

[asraa]

yes! at least to me that's the right approach

[j2kun]

I don't think this is a solution to #1992, with or without dynamic legality. The problem is that the type converter didn't support a type that showed up in the input, and that caused a crash because the context-aware dialect conversion stuff is just not super robust in that way. Making arith illegal (or making certain ops dynamically illegal) won't solve the underlying problem, and instead would just make this pass crash on a variety of inputs because arith ops are supported in the output.

I'm inclined to think the "right" solution here is, as a first step of the pass, to walk the IR and verify all types encountered are supported by the type converter. E.g., use walkAndValidateTypes on the root operation and the condition can be "is this type in a list of approved types," which is manually synced with the type converter.

[asraa]

Ah, I didn't realize the failure was caused by the IndexType. I see Alex mentioned that in the issue.

E.g., use walkAndValidateTypes on the root operation and the condition can be "is this type in a list of approved types," which is manually synced with the type converter.

Shouldn't the fix happen in the framework to prevent users from accidentally not adding the walker?

[j2kun]

I'm not sure if the framework can tell what types are supported in the caller's type converter...

[AlexanderViand]

But even if it's not supported, shouldn't it fail with something slightly more elegant than segfault?

[j2kun]

I'm just saying I don't think we can solve this generally in the framework, because what is supported is dependent on the user of the framework. Instead we'll have to do a slightly more specific but still easy thing (walk the IR and validate types), and it will avoid the segfault

[ai-mannamalai]

@asraa @j2kun - made an update to check for invalid types; how does it look ?

[asraa]

Thank you! The walker looks fine, but I suggested another fix where instead we use the info from the registered type conversion instead of using a list of invalid types (that might go stale)

[asraa]

@j2kun why can't something like this work, where we call on the type converter to see if a valid conversion for the value is registered?

    // check for invalid types and bail out of the pass
    auto isValidValue = [&](const Value value) -> LogicalResult {
      auto contextualAttr = typeConverter.getContextualAttr(value);
      if (failed(contextualAttr)) {
        return failure();
      }
      Type result =
          typeConverter.convertType(value.getType(), contextualAttr.value());
      return result != nullptr ? success() : failure();
    };
    if (!walkAndValidateValues(module, isValidValue).succeeded()) {
      module->emitError() << "invalid type in module";
      return signalPassFailure();
    }

[ai-mannamalai]

let me integrate this; @asraa and see

[ai-mannamalai]

Looks like its not OK; even standard regression fail and we need some work done for additional IR tests. From: Jeremy Kun ***@***.***> Date: Friday, August 15, 2025 at 4:03 PM To: google/heir ***@***.***> Cc: Muthu Annamalai ***@***.***>, Author ***@***.***> Subject: [EXTERNAL] Re: [google/heir] #1992: Convert to Ciphertext Semantics Update (PR #2073) @j2kun requested changes on this pull request.

________________________________ In lib/Transforms/ConvertToCiphertextSemantics/ConvertToCiphertextSemantics.cpp<#2073 (comment)>:

@@ -1064,6 +1064,22 @@ struct ConvertToCiphertextSemantics

RewritePatternSet patterns(context); ConversionTarget target(*context); + + // check for invalid types on values and bail out of the pass + auto isValidValue = [&](const Value value) -> LogicalResult { + auto contextualAttr = typeConverter.getContextualAttr(value); + if (failed(contextualAttr)) { + return failure(); + } + Type result = + typeConverter.convertType(value.getType(), contextualAttr.value()); + return result != nullptr ? success() : failure(); + }; + if (!walkAndValidateValues(module, isValidValue).succeeded()) { + module->emitError() << "invalid type in module"; This looks OK, but could we possibly test this? Maybe add a test with an IR that uses some unsupported type like memref and tries to run this pass? Note you can use -verify-diagnostics to assert error messages are correctly generated. — Reply to this email directly, view it on GitHub<#2073 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/APGLTDFZWA2ACHL327O7IYT3NZRK7AVCNFSM6AAAAACDHFJHCKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTCMRVGMZDKMRWG4>. You are receiving this because you authored the thread.

[ai-mannamalai]

memref present in the IR is trapped by the above code; e.g. run

$ bazelisk run --//:enable_openmp=0 //tools:heir-opt -- --mlir-to-cggi=data-type=Integer --scheme-to-tfhe-rs --convert-to-ciphertext-semantics `pwd`/tests/Examples/tfhe_rust_hl/cpu/add_round_key.mlir

need to see if the checks are overzealous which causes some regression test failure

[ai-mannamalai]

I find the ops like arith.mulf, arith.addf are being flagged by the new pass. Is this expected ?

[ai-mannamalai]

added a new test; testing against the regression suite should pass now

[ai-mannamalai]

errors look like,

/home/heiruser/heir/tests/Examples/tfhe_rust_hl/cpu/add_round_key.mlir:0:0: error: invalid attribute at value:
<block argument> of type 'memref<16x!tfhe_rust.eui8>' at index: 1
/home/heiruser/heir/tests/Examples/tfhe_rust_hl/cpu/add_round_key.mlir:0:0: error: invalid type in module

[j2kun]

This test is a bit confusing, because it runs the full CGGI pipeline and converts to tfhe_rs before applying the convert-to-ciphertext-semantics pass (which is for non-CGGI schemes).

Could you instead try cloning a test like https://github.com/google/heir/blob/1263704af22e8a0c85a1b513e26f1eeb885fb965/tests/Transforms/convert_to_ciphertext_semantics/tensor_extract.mlir and replacing the tensor types/ops with memref equivalents? Then use --verify-diagnostics as in

heir/tests/Dialect/Random/IR/verifier.mlir

Line 1 in 1263704

// RUN: heir-opt %s --verify-diagnostics 2>&1

to ensure messages from emitError are properly emitted.

[ai-mannamalai]

sure; let me take a look