Converting MLIR switch with no cases to llvm-ir asserts

Hello!

I have recently encountered an assertion from MLIR’s LLVM-to-llvm-ir
conversion and was wondering if anyone has any insights ?
I do apologize if I am missing something obvious and this is in-fact intentional.

Note that I am using an old MLIR version so this might have been fixed already
(IF it is indeed an issue) but I wasn’t able to find anything on GitHub.

Here is the input:

// input.mlir
llvm.func @main(%arg0: i32) {
  llvm.switch %arg0, ^bb1 [
  ]
^bb1:  // pred: ^bb0
  llvm.br ^bb2
^bb2:  // pred: ^bb1
  llvm.return
}

And here is the command I am running:

mlir-translate ./input.mlir --mlir-to-llvmir

I am getting an assertion as follows:

llvm-project/llvm/include/llvm/ADT/Optional.h:196: T& llvm::optional_detail::OptionalStorage<T, true>::getValue() & [with T = mlir::ElementsAttr]: Assertion `hasVal' failed.

Which I believe comes from HERE.

(The link above points to the last commit that looks the same as my version on that particular line … however I believe that a similar assertion will be thrown from HERE in the latest master)

Thank you in advance!

Thanks,
Thanasis.

First: this looks like a missing canonicalization to remove the switch entirely and merge the blocks.

We should still not assert here, this is likely a bug to be fixed. That said I don’t know if this is allowed in the LLVM IR?

Thank you @mehdi_amini !
Correct, this should definitely be canonicalized.
We had disabled region simplification though due to another bug which I believe
is related to this: [mlir-opt] -canonicalize fails · Issue #50791 · llvm/llvm-project · GitHub
Since I had an older version of LLVM I couldn’t use that yet thus I hit this issue
during my experiments.

But we can easily optimize this ourselves actually so it’s definitely not a blocker.

That said I don’t know if this is allowed in the LLVM IR?

Well, apparently it works (I just tried it).

I wonder if it’s OK for me to try and fix that issue as an exercise ?
Is it OK if I open a ticket about this ?

Thanks again,
Thanasis.

Yes absolutely! Ideally we would support importing and exporting this from/to LLVM IR. Thanks for giving it a try, and let us know if you need help!

1 Like