This is an archive of the discontinued LLVM Phabricator instance.

[Flang] Upstream conversion of the XRebox Op
ClosedPublic

Authored by kiranchandramohan on Nov 29 2021, 7:44 AM.

Details

Summary

The XRebox Op is formed by the codegen rewrite which makes it easier to
convert the operation to LLVM. The XRebox op includes the information
from the rebox op and the associated slice, shift, and shape ops.

During the conversion process a new descriptor is created for reboxing.

Co-authored-by: Jean Perier <jperier@nvidia.com>
Co-authored-by: Eric Schweitz <eschweitz@nvidia.com>
Co-authored-by: Val Donaldson <vdonaldson@nvidia.com>

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptNov 29 2021, 7:44 AM
Herald added a subscriber: mehdi_amini. · View Herald Transcript
kiranchandramohan requested review of this revision.Nov 29 2021, 7:44 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 29 2021, 7:44 AM
flang/lib/Optimizer/CodeGen/CodeGen.cpp
1747

I had to bring this function inside the pattern since the compiler was complaining that it is not able to find this function. But it seems to work fine in the fir-dev branch. https://github.com/flang-compiler/f18-llvm-project/blob/b111a42388dd1516ff8beb126cf53cd573737b4a/flang/lib/Optimizer/CodeGen/CodeGen.cpp#L92

Any ideas?

1825

@jeanPerier Here and in a few other places we were using the original operands and not the converted operands. This caused some issue when I cherry-picked the patch to llvm-project/flang. In discourse it was pointed out that this is because we are not using the converted operands (https://llvm.discourse.group/t/issue-in-conversion-from-index-to-i64/4853). Is there any reason we were using the original operands?
For getting the patch to work I had to switch to the converted operands.

1865

Is there a better way to do this valuerange creation?

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

More tests are required for derived types and character.

clementval added inline comments.Nov 29 2021, 7:53 AM
flang/lib/Optimizer/CodeGen/CodeGen.cpp
214
1483

I introduced some constexpr for the different element in dim in this patch https://reviews.llvm.org/D114148#change-7X7u66OPbYbl but it is not in yet.

Feel free to base your patch on mine or just bring in these 3 constexpr so the magic number will have a meaning.

static constexpr unsigned kDimLowerBoundPos = 0;
static constexpr unsigned kDimExtentPos = 1;
static constexpr unsigned kDimStridePos = 2;
1488
1493
1759
1782–1783
1817
1819–1822
1825
1828
1842
1844
1845
1876
1881
jeanPerier added inline comments.Nov 29 2021, 9:18 AM
flang/lib/Optimizer/CodeGen/CodeGen.cpp
1825

Is there any reason we were using the original operands?

No, I think I just did not realize I was not using the converted ones and it just worked. Thanks for catching this and fixing it.

Specify types, Add a test for derived type and character.

kiranchandramohan marked 15 inline comments as done.Dec 3 2021, 8:06 AM

Addressed review comments.

flang/lib/Optimizer/CodeGen/CodeGen.cpp
1819–1822

inputStrides is a valueRange. I have kept it auto since it is an iterator for a valuerange.

This revision is now accepted and ready to land.Dec 6 2021, 6:30 AM
kiranchandramohan marked an inline comment as done.

Restore getVoidPtrType to its original location.

This revision was automatically updated to reflect the committed changes.