This is an archive of the discontinued LLVM Phabricator instance.

[fir] Add fir.gentypedesc conversion
ClosedPublic

Authored by clementval on Nov 12 2021, 6:19 AM.

Details

Summary

Add conversion pattern for the GenTypeDescOp.
Convert to a global constant with an addressof.

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

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

Diff Detail

Event Timeline

clementval created this revision.Nov 12 2021, 6:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 12 2021, 6:19 AM
Herald added a subscriber: mehdi_amini. · View Herald Transcript
clementval requested review of this revision.Nov 12 2021, 6:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 12 2021, 6:19 AM
flang/lib/Optimizer/CodeGen/CodeGen.cpp
685

Nit: Can you add a bit more documentation on the conversion (global + addressof) and why is it a global (when it was not in FIR).

schweitz added inline comments.Nov 15 2021, 11:52 AM
flang/lib/Optimizer/CodeGen/CodeGen.cpp
685

This prototype lowering is alluding to one possible implementation.

In FIR, one need not know the exact layout and contents of a "type descriptor". It is an abstraction that can be used to perform type tests, like "isa", perform virtual dispatch (for type bound procedures), etc. (This is like the box abstraction, which allows a box to be lowered to any number of "descriptor" formats, depending on the target runtime, etc.)

When lowering from FIR to LLVM, some sort of (constant) data structure must either be present (and referenced) or be created. Initially there was nothing, so the prototype created a dummy implementation, which is what we have here. The front end has since evolved and is generating its own data structures. (These can be found in the FIR dumps of tests with derived types, for example.) It is not entirely clear how this will be brought together.

This prototype could be replaced with a TODO.

Replace the prototype lowering with not implmented yet.

LGTM.

Nit: Are the additional headers required?

This revision is now accepted and ready to land.Nov 16 2021, 3:45 AM
clementval marked 2 inline comments as done.Nov 16 2021, 4:38 AM

LGTM.

Nit: Are the additional headers required?

Not anymore. I'm gonna remove them before pushing.

Rebase + remove extra includes

This revision was automatically updated to reflect the committed changes.