[MLIR]code design problems with conversion pass

MLIR
1

Hello, I have encountered some problems in three conversion passes with same error with message failed to legalize operation 'xxx' that was explicitly marked illegal.

The reasons for this error are similar:
the code target.addIllegalDialect<XXX>() makes some operations cannot be converted are marked illegal.

#bug 1: ConvertMathToLibmPass
When i use cmd mlir-opt -convert-math-to-libm, i got an error : failed to legalize operation.
I think the problem is that the pass -convert-math-to-libm does not legitimize some operations (e.g., math.powf,math.floor,math.log), and this code target.addIllegalDialect<math::MathDialect>() in ConvertMathToLibmPass makes these operations are marked illegal.

MathToLibm.cpp:

void ConvertMathToLibmPass::runOnOperation() {
  target.addIllegalDialect<math::MathDialect>();     
  ...
}

Steps to reproduce:

mlir-opt -convert-math-to-libm test.mlir

test case:

func.func @pow_noop(%arg0: f32, %arg1 : vector<4xf32>) -> (f32, vector<4xf32>) {
  %c = arith.constant 1.0 : f32
  %v = arith.constant dense <1.0> : vector<4xf32>
  %0 = math.powf %arg0, %c : f32
  %1 = math.powf %arg1, %v : vector<4xf32>
  return %0, %1 : f32, vector<4xf32>
}

The error message is like:

/home/ty/test.mlir:8:8: error: failed to legalize operation 'math.powf' that was explicitly marked illegal
  %0 = math.powf %arg0, %c : f32
       ^

#bug 2: BufferizationToMemRefPass
BufferizationToMemRefPass can only handle bufferization.clone, and set the BufferizationDialect illegal after conversion by target.addIllegalDialect<bufferization::BufferizationDialect>(). The operation bufferization.to_memref cannot be converted and is marked as illegal, thus causing this error.

BufferizationToMemRef.cpp:

void mlir::populateBufferizationToMemRefConversionPatterns(
    RewritePatternSet &patterns) {
   patterns.add<CloneOpConversion>(patterns.getContext());
}
void runOnOperation() override {
   populateBufferizationToMemRefConversionPatterns(patterns);
   target.addIllegalDialect<bufferization::BufferizationDialect>();          
   ...
}

Steps to reproduce:

mlir-opt -convert-bufferization-to-memref test.mlir

test case:

func.func @tensor_load_of_buffer_cast(%arg0: tensor<?xf32>) -> tensor<?xf32> {
  %0 = bufferization.to_memref %arg0 : memref<?xf32>
  %1 = bufferization.to_tensor %0 : memref<?xf32>
  return %1 : tensor<?xf32>
}

error message:

/home/ty/test.mlir:4:8: error: failed to legalize operation 'bufferization.to_memref' that was explicitly marked illegal
  %0 = bufferization.to_memref %arg0 : memref<?xf32>
       ^

#bug 3: ConvertSPIRVToLLVMPass

void ConvertSPIRVToLLVMPass::runOnOperation() {
  target.addIllegalDialect<spirv::SPIRVDialect>();
  ...
}

Steps to reproduce:

mlir-opt -convert-spirv-to-llvm test.mlir

test case:

module {
 func.func @const() -> () {
  %8 = spv.Constant [dense<3.0> : vector<2xf32>] : !spv.array<1xvector<2xf32>>
  return
  }
}

error message:

/home/ty/test.mlir:3:8: error: failed to legalize operation 'spv.Constant' that was explicitly marked illegal
  %8 = spv.Constant [dense<3.0> : vector<2xf32>] : !spv.array<1xvector<2xf32>>
       ^

I think there may be some problems with the code design.
I have committed the three bugs mentioned above in github. Welcome to verify and discuss together.

Thanks,
Bealle

2

Passes in MLIR are generally allowed to error out if their input doesn’t satisfy certain invariants, like presence or absence of certain ops. It all depends on the contract that the pass provides, e.g., A-to-B conversion passes may have a contract that there are no operations from the A dialect in the resulting IR. If they are unable to achieve this, an error is justified.

For conversion passes, it may also be reasonable to perform a partial conversion and leave the unhandled ops as is. This is up for maintainers of the respective passes.

Side note: some people view convert-A-to-B passes as testing infrastructure for the conversion patterns and expect MLIR clients to use the patterns directly instead of calling the passes.

3

I agree in the general sense, in the case of the “convert-math-to-libm” pass I’m not convinced we should error here: either we’re missing implementations for these conversions or we should mark these legal.