This is an archive of the discontinued LLVM Phabricator instance.

[fir] Add fir.box_rank, fir.box_addr, fir.box_dims and fir.box_elesize conversion
ClosedPublic

Authored by clementval on Nov 10 2021, 1:18 AM.

Details

Summary

This patch adds conversion for basic box operations that extract
information from the box.

This patch is part of the upstreaming effort from fir-dev branch.

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

Diff Detail

Event Timeline

clementval created this revision.Nov 10 2021, 1:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 10 2021, 1:18 AM
Herald added a subscriber: mehdi_amini. · View Herald Transcript
clementval requested review of this revision.Nov 10 2021, 1:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 10 2021, 1:18 AM
awarzynski added inline comments.
flang/lib/Optimizer/CodeGen/CodeGen.cpp
32–35

It's great to see some non-magic values used here! Do these numbers correspond to https://github.com/llvm/llvm-project/blob/99ad2079d452f587be050b3867e0ed4856335fb2/flang/lib/Optimizer/CodeGen/TypeConverter.h#L105-L116? If yes, could these be used for the type converter too? Also, could you add a comment here that would explain the origins of these indices? Ta!

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

Shouldn't this be written in a target agnostic way? Like here: https://github.com/llvm/llvm-project/blob/99ad2079d452f587be050b3867e0ed4856335fb2/flang/test/Fir/types-to-llvm.fir#L38 Same comment for other tests here.

653–656

DELETEME?

clementval added inline comments.Nov 10 2021, 2:02 AM
flang/lib/Optimizer/CodeGen/CodeGen.cpp
32–35

Yes they are the same. Let me do a quick patch to move those value in TypeConverter and use them. I'll base this patch on it.

clementval marked 3 inline comments as done.

Address review comments

flang/lib/Optimizer/CodeGen/CodeGen.cpp
32–35

Done in D113553

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

Yeah might be safer.

awarzynski accepted this revision.Nov 10 2021, 5:12 AM

Thanks for the updates,

Makes sense. The remaining comments are mostly nits, so I'm accepting as is.

flang/lib/Optimizer/CodeGen/CodeGen.cpp
54

[nit] It's clear that it's a method. Construct is the key information here.

73

[nit]

111–113

c0 is the offset to get the relevant struct (i.e. to get the base address), and c1 is the offset within the struct (i.e. to get the element address), right?

123

What's cv?

228

DELETEME

This is an implementation detail from getRankFromBox.

This revision is now accepted and ready to land.Nov 10 2021, 5:12 AM
clementval marked 5 inline comments as done.

Address review comments

flang/lib/Optimizer/CodeGen/CodeGen.cpp
73

This is always 0.

111–113

Yes that's right. c0 is always 0 when accessing the box.

123

constant values.

228

My bad. Was left from the code when there was not constant for the different positions.

This revision was landed with ongoing or failed builds.Nov 10 2021, 6:35 AM
This revision was automatically updated to reflect the committed changes.