This is an archive of the discontinued LLVM Phabricator instance.

[flang][CodeGen] Transform `fir.boxchar_len` to a sequence of LLVM MLIR
ClosedPublic

Authored by awarzynski on Nov 12 2021, 5:30 AM.

Details

Summary

This patch extends the FIRToLLVMLowering pass in Flang by adding a
hook to transform fir.boxchar_len to a sequence of LLVM MLIR
instructions.

This is part of the upstreaming effort from the fir-dev branch in [1].

[1] https://github.com/flang-compiler/f18-llvm-project

Originally written by:
Co-authored-by: Eric Schweitz <eschweitz@nvidia.com>

Diff Detail

Event Timeline

awarzynski created this revision.Nov 12 2021, 5:30 AM
awarzynski requested review of this revision.Nov 12 2021, 5:30 AM

IIUC, both forms are correct (based on the operation definition:

%0 = fir.boxchar_len %arg0 : (!fir.boxchar<4>) -> i64
%0 = fir.boxchar_len %arg0 : (!fir.boxchar<4>) -> i32

As the length inside !fir.boxchar will be either i32 or i64 (this depends on the target), !fir.boxchar may require an integer cast on the result. Please see the tests for examples.

@schweitz , could you confirm whether this is correct? There was no integer cast on fir-dev, but I added it here. But perhaps I'm missing something here?

IIUC, both forms are correct (based on the operation definition:

%0 = fir.boxchar_len %arg0 : (!fir.boxchar<4>) -> i64
%0 = fir.boxchar_len %arg0 : (!fir.boxchar<4>) -> i32

As the length inside !fir.boxchar will be either i32 or i64 (this depends on the target), !fir.boxchar may require an integer cast on the result. Please see the tests for examples.

@schweitz , could you confirm whether this is correct? There was no integer cast on fir-dev, but I added it here. But perhaps I'm missing something here?

Did you try to add the cast on fir-dev to see the impact on the tests we have there?

Did you try to add the cast on fir-dev to see the impact on the tests we have there?

Yes, all tests pass for me.

clementval added inline comments.Nov 15 2021, 2:14 PM
flang/lib/Optimizer/CodeGen/CodeGen.cpp
1534

This line seems to be useless.

IIUC, both forms are correct (based on the operation definition:

%0 = fir.boxchar_len %arg0 : (!fir.boxchar<4>) -> i64
%0 = fir.boxchar_len %arg0 : (!fir.boxchar<4>) -> i32

As the length inside !fir.boxchar will be either i32 or i64 (this depends on the target), !fir.boxchar may require an integer cast on the result. Please see the tests for examples.

@schweitz , could you confirm whether this is correct? There was no integer cast on fir-dev, but I added it here. But perhaps I'm missing something here?

fir.boxchar_len is polymorphic on the return type. Any integer type is fine and must be dealt with when generating LLVM IR.

flang/lib/Optimizer/CodeGen/CodeGen.cpp
1534

Bad merge?

Remove unused code

clang-format

awarzynski added inline comments.Nov 16 2021, 1:02 AM
flang/lib/Optimizer/CodeGen/CodeGen.cpp
1534

I have deleted that line and reformatted this bit. Thanks for pointing this out!

clementval accepted this revision.Nov 16 2021, 1:19 AM

LG

flang/lib/Optimizer/CodeGen/CodeGen.cpp
1534

nit: maybe having a constexpr with a proper name could help here instead of 1 or maybe a small comment.

This revision is now accepted and ready to land.Nov 16 2021, 1:19 AM
awarzynski added inline comments.Nov 16 2021, 5:40 AM
flang/lib/Optimizer/CodeGen/CodeGen.cpp
1534

That's a good point - I'll update before merging.