Failure to lower scf::IfOp that yields local values

MLIR
1

Hello,

Here is the issue as I understand it currently. Assuming we have the following mlir fragment:

%true = constant 
%value = foo.op ... : si16
%if_ret:2 = scf.if %cond -> (i1, si16) {
  scf.yield %true, %value : i1, si16
} else {
  scf.yield %false, %zero : i1, si16
}

where we may assume that all these SSA values are defined prior to this IfOp of course. This is totally fine and works as expected. However, if we move %value inside the IfOp as follows:

%if_ret:2 = scf.if %cond -> (i1, si16) {
  %value = foo.op ... : si16
  scf.yield %true, %value : i1, si16
} else {
  scf.yield %false, %zero : i1, si16
}

then this will throw the error:

error: failed to legalize operation 'scf.if'

Even though I have defined a converter for my custom foo.op which works in the first case, however, it doesn’t seem to kick-in in the second case.
In fact, even if I call target.addLegalOp<FooOp>() I still get the same error.

P.S.: I am using the LLVMConversionTarget

Thank you in advance!

Thanasis.

2

I can’t speak to this specific failure, but you’ll probably want to run this with --debug-only=dialect-conversion, which will provide helpful output about all the patterns the conversion framework attempts to apply and their success/failure. See Debugging Tips - MLIR and Dialect Conversion - MLIR

3

This is likely a bug, but can you provide a reproducer test case? (a minimal .mlir file that runs with mlid-opt)

4

Thank You!

The --debug-only=dialect-conversion definitely helped!

I think I have narrowed down the issue. The issue is related to type materialization so in order to reproduce you would need an op that may return a non-signless integer type like si16 or ui16. I’ll try to create a reproducible test asap.

In the meantime, what happens is that the converter will produce an llvm.mlir.cast (or LLVM::DialectCastOp) to cast the si16/ui16 types to i16. It seems that this materialization falls into some kind of infinite loop. Here is the relevant part of the log:

Legalizing operation : 'std.br'(0x416d6b0) {
  "std.br"(%5, %8)[^bb3] : (i1, si16) -> ()

  * Fold {
  } -> FAILURE : unable to fold

  * Pattern : 'std.br -> ()' {
    ** Insert  : 'llvm.mlir.cast'(0x415eb98)
    ** Insert  : 'llvm.br'(0x4172b40)
    ** Replace : 'std.br'(0x416d6b0)

    //===-------------------------------------------===//
    Legalizing operation : 'llvm.mlir.cast'(0x415eb98) {
      %9 = "llvm.mlir.cast"(%8) : (si16) -> i16

      * Fold {
      } -> FAILURE : unable to fold

      * Pattern : 'llvm.mlir.cast -> ()' {
        ** Insert  : 'llvm.mlir.cast'(0x416d568)
        ** Replace : 'llvm.mlir.cast'(0x415eb98)

        //===-------------------------------------------===//
        Legalizing operation : 'llvm.mlir.cast'(0x416d568) {
          %9 = "llvm.mlir.cast"(%8) : (si16) -> i16

          * Fold {
          } -> FAILURE : unable to fold

          * Pattern : 'llvm.mlir.cast -> ()' {
          } -> FAILURE : pattern was already applied
        } -> FAILURE : no matched legalization pattern
        //===-------------------------------------------===//
     } -> FAILURE : generated operation 'llvm.mlir.cast'(0x000000000416D568) was illegal
   } -> FAILURE : pattern failed to match
 } -> FAILURE : no matched legalization pattern
 //===-------------------------------------------===//
} -> FAILURE : generated operation 'llvm.mlir.cast'(0x000000000415EB98) was illegal

As you can see, a daisy-chain of llvm.mlir.cast ops is created which is thankfully stopped by the infrastructure (it detects whether a given pattern is applied already). Otherwise, I think it might have fallen into an infinite loop.

5

Here is a reproducible test written using the std and scf dialects only:

// file: test.mlir
func @bar(%arg: si16) -> si16 {
  return %arg : si16
}

func @foo(%arg: si16) -> si16 {
  %0 = constant true
  %1 = call @bar(%arg) : (si16) -> si16
  %2:2 = scf.if %0 -> (i1, si16) {
    scf.yield %0, %1 : i1, si16
  } else {
    // This value will trigger the issue
    %3 = call @bar(%arg) : (si16) -> si16
    scf.yield %0, %3 : i1, si16
  }
  return %arg : si16
}

If you execute the following:

mlir-opt test.mlir --convert-scf-to-std | mlir-opt --convert-std-to-llvm

then it works!

However, if you write a pass as follows:

struct testConverter : public testConverterBase<testConverter> {
  void runOnOperation() final;
};

void testConverter::runOnOperation() {
  auto module = getOperation();
  auto *ctx = module.getContext();

  LLVMConversionTarget target(*ctx);
  target.addLegalOp<ModuleOp, ModuleTerminatorOp>();

  LLVMTypeConverter typeConverter(ctx);
  OwningRewritePatternList patterns;
  populateStdToLLVMConversionPatterns(typeConverter, patterns);
  populateLoopToStdConversionPatterns(patterns, ctx);

  if (failed(applyFullConversion(module, target, std::move(patterns))))
    signalPassFailure();
}

std::unique_ptr<mlir::OperationPass<ModuleOp>>
cslang::createTestConverter() {
  return std::make_unique<TestConverter>();
}

then you will get the error I reported earlier.
The code above is missing boilerplate but hopefully you’ll get the idea and will be able to reproduce.

6

You may find this helpful, it seems to me the same problem I encountered some time ago.

7

Conversion to LLVM does not work on signed or unsigned integers, only signless integers. The input code uses si16, this will fail the conversion.

8

Thank you all! Alex, I think that settles it for me. Adjusting my foo custom dialect to be signless solved the issue. However, I was wondering … should I still file a bug report for this to make the failure more clear (as opposed to failing subtly) ? For example, should the type materializer throw an error when a signed/unsigned int is encountered instead of trying to convert it assuming its OK ?

9

That would require non-trivial changes, and cannot say outright if it is even feasible.

  1. Most conversion passes are partial conversions, meaning that as long as the result does not contain ops declared illegal in the conversion target, they succeed. The expectation is that some further conversion will deal with the remaining ops. This is a conscious design decision, otherwise we risk running into the monolithic all-to-one-dialect conversions quickly.
  2. The fact that a specific conversion does not know how to convert specific types does not and should not mean conversion failure. Some patterns may fail to match, other patterns may be more forgiving and adapt.
  3. Materialization hooks are there to inject opaque casts. They are not currently configured to cast from (un)signed to singless integers but they could be. And some further pass may eliminate those.

So the actual error would be something like “the conversion infra couldn’t convert the type, which led to all potentially applicable patterns not being applied (list of reasons for each pattern), which led to source materialization, which injected an llvm.mlir.cast op of a form that is illegal; abort”. This kind of logic reasoning feels hard to build.