[PATCH] D72542: [Flang] add a band-aid to support the creation of mutually recursive types when lowering to LLVM IR

[Eric Schweitz via Phabricator via llvm-commits]

schweitz marked an inline comment as done.
schweitz added inline comments.


================
Comment at: mlir/test/Dialect/LLVMIR/roundtrip.mlir:222
+
+// XXXCHECK: llvm.func @recursive_type(!llvm<"%a = type { %a* }">)
+//llvm.func @recursive_type(!llvm<"%a = type { %a* }">)
----------------
jdoerfert wrote:
> schweitz wrote:
> > ftynse wrote:
> > > jdoerfert wrote:
> > > > A "FIXME/TODO" and an actual `CHECK` might be better here. The FIXME explains the problem and the `CHECK` makes sure we see changes to whatever we are doing.
> > > +1
> > Can someone provide a pointer to something like this? I'm having a hard time envisioning a test that tests for a TODO item.
> ```
> TODO: This should be llvm.func @recursive_type(!llvm<"%a = type { %a* }">)
> CHECK: <here check what it is right now>
> ```
Running this test as `mlir-opt %s | mlir-opt | FileCheck %s` would result in the following output.

```
mlir/test/Dialect/LLVMIR/roundtrip.mlir:223:34: error: expected end of string
llvm.func @recursive_type(!llvm<"%a = type { %a* }">)
                                 ^
mlir/test/Dialect/LLVMIR/roundtrip.mlir:3:17: error: CHECK-LABEL: expected string not found in input
// CHECK-LABEL: func @ops(%arg0: !llvm.i32, %arg1: !llvm.float)
                ^
<stdin>:3:1: note: scanning from here
module {
^
```

That seems an undesirable option.

Another approach would be to add a separate test altogether, a test that expects the tool to error and terminate.


================
Comment at: mlir/test/Dialect/LLVMIR/roundtrip.mlir:223
+// XXXCHECK: llvm.func @recursive_type(!llvm<"%a = type { %a* }">)
+//llvm.func @recursive_type(!llvm<"%a = type { %a* }">)
----------------
ftynse wrote:
> Could we have test for translating such types to LLVM? Normally, we should be able to parse the recursive type and see it after translation. Somewhere in test/Target/llvmir.mlir would be the right place.
The title of this diff was meant to reflect the transitory nature of this patch. Parser work needs to be done to finish this. It is still undecided the exact nature of that parser work, as far as I understand.

This patch was being driven by merging the MLIR dependence into the F18/Flang project which was itself trying to be merged into the LLVM monorepo. It looks like all of these merges are on hold for now, so the urgency to have some sort of solution has been alleviated.

We still need real support for recursive-types in the LLVM-IR dialect of MLIR at some point (to support Fortran in our case).



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72542/new/

https://reviews.llvm.org/D72542