Page MenuHomePhabricator

[flang] Fix XArrayCoorOp conversion for index type slices
ClosedPublic

Authored by peixin on May 19 2022, 5:12 AM.

Details

Summary

The previous XArrayCoorOp conversion forgot to change getting the
operands from OpAdaptor for upper bound and step of slice. This leads to
the fail of incompatible of types of codegen when slices are index type.

Diff Detail

Event Timeline

peixin created this revision.May 19 2022, 5:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 19 2022, 5:12 AM
peixin requested review of this revision.May 19 2022, 5:12 AM
peixin added a comment.EditedMay 19 2022, 5:24 AM

@jeanPerier @schweitz When do you plan to upstream the IO runtime functions to the libraries?

$ cat case.f90 
program m
 integer :: i = 1
 print *, i
end
$ flang-new case.f90 && ./a.out
/usr/bin/ld: /usr/lib/gcc/aarch64-linux-gnu/7.3.0/../../../../lib64/Scrt1.o: in function `_start':
(.text+0x18): undefined reference to `main'
/usr/bin/ld: (.text+0x1c): undefined reference to `main'
/usr/bin/ld: /tmp/case-f0bed5.o: in function `_QQmain':
/home/..././case.f90:3: undefined reference to `_FortranAioBeginExternalListOutput'
/usr/bin/ld: /home/..././case.f90:3: undefined reference to `_FortranAioOutputInteger32'
/usr/bin/ld: /home/..././case.f90:3: undefined reference to `_FortranAioEndIoStatement'
flang-new: error: linker command failed with exit code 1 (use -v to see invocation)

Currently, I found some runtime functions not in fir-dev such as _FortranARaggedArrayAllocate. Previously, I can get llvm IR from LLVM main and link the libraies from fir-dev to have end-to-end tests for LLVM main. But now, it does not work for some cases now.

Usually, I prefer end-to-end tests. With IO support, I can start that in LLVM main. The bug in this patch is not in fir-dev, although there is another bug for XArrayCoorOp in fir-dev.

@jeanPerier @schweitz When do you plan to upstream the IO runtime functions to the libraries?

The runtime IO libraries are upstreamed already. https://github.com/llvm/llvm-project/blob/602f81ec336330f97e22442b98035c6f007cac6d/flang/runtime/io-api.cpp#L194
Developers can use the flag -flang-experimental-exec for linking with the libraries.

$ cat case.f90 
program m
 integer :: i = 1
 print *, i
end
$ flang-new case.f90 && ./a.out
/usr/bin/ld: /usr/lib/gcc/aarch64-linux-gnu/7.3.0/../../../../lib64/Scrt1.o: in function `_start':
(.text+0x18): undefined reference to `main'
/usr/bin/ld: (.text+0x1c): undefined reference to `main'
/usr/bin/ld: /tmp/case-f0bed5.o: in function `_QQmain':
/home/..././case.f90:3: undefined reference to `_FortranAioBeginExternalListOutput'
/usr/bin/ld: /home/..././case.f90:3: undefined reference to `_FortranAioOutputInteger32'
/usr/bin/ld: /home/..././case.f90:3: undefined reference to `_FortranAioEndIoStatement'
flang-new: error: linker command failed with exit code 1 (use -v to see invocation)

Currently, I found some runtime functions not in fir-dev such as _FortranARaggedArrayAllocate. Previously, I can get llvm IR from LLVM main and link the libraies from fir-dev to have end-to-end tests for LLVM main. But now, it does not work for some cases now.

Hopefully with the flag mentioned above you can use the libraries from llvm-project/flang.

Usually, I prefer end-to-end tests. With IO support, I can start that in LLVM main. The bug in this patch is not in fir-dev, although there is another bug for XArrayCoorOp in fir-dev.

Apologies, this was probably caused while switching from operands to converted operands in the upstream code.

The runtime IO libraries are upstreamed already. https://github.com/llvm/llvm-project/blob/602f81ec336330f97e22442b98035c6f007cac6d/flang/runtime/io-api.cpp#L194
Developers can use the flag -flang-experimental-exec for linking with the libraries.

Hopefully with the flag mentioned above you can use the libraries from llvm-project/flang.

Thanks. This works.

Apologies, this was probably caused while switching from operands to converted operands in the upstream code.

Yes. That's what I also noticed. Anyway, using operands directly rather than converted operands in dir-dev is not correct.

Apologies, this was probably caused while switching from operands to converted operands in the upstream code.

Yes. That's what I also noticed. Anyway, using operands directly rather than converted operands in dir-dev is not correct.

OK. I think this is just a missed case. It works in fir-dev but due to some core-MLIR change, it does not work upstream.

LGTM. Please wait for @schweitz or @jeanPerier.

flang/lib/Optimizer/CodeGen/CodeGen.cpp
2077

Nit: are these (indexOps, shapeOps, shiftOps, sliceOps) not used anymore? Can these variables be removed?

This revision is now accepted and ready to land.May 19 2022, 7:26 AM
peixin updated this revision to Diff 430718.May 19 2022, 9:30 AM

Thanks @kiranchandramohan for the notice. Remove the unused code.

schweitz added inline comments.May 27 2022, 11:57 AM
flang/test/Fir/convert-to-llvm.fir
2060

Can we add some additional tests? An array of 3 dimensions?

It would be good to exercise an array of record type, fir.type, with a subcomponent path and type parameters. Especially if we're not already doing that.

peixin updated this revision to Diff 432706.May 28 2022, 1:46 AM

Add two test cases.

flang/test/Fir/convert-to-llvm.fir
2060

Yes. These two kinds of tests were missed. Add them now.

schweitz accepted this revision.Mon, Jun 6, 8:12 AM

Thanks for adding the tests.

This revision was automatically updated to reflect the committed changes.