Page MenuHomePhabricator

[flang][fir][NFC] Move CharacterType and BoxCharType to TableGen type definition
ClosedPublic

Authored by clementval on Feb 10 2021, 12:36 PM.

Details

Summary

This patch is a follow up of D96422 and move CharacterType and BoxCharType to
TableGen.

Diff Detail

Event Timeline

clementval requested review of this revision.Feb 10 2021, 12:36 PM
clementval created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 10 2021, 12:36 PM
clementval added inline comments.Feb 10 2021, 12:40 PM
flang/include/flang/Optimizer/Dialect/FIRTypes.td
51

@mehdi_amini @rriddle I'm currently forcing this type to be generated before BoxCharType because it depends on it. Is there a better way to achieve this than adding a prefix (A_)?

clementval retitled this revision from [flang][fir][NFC] Move CharacterType and BoxCharType to to TableGen type definition to [flang][fir][NFC] Move CharacterType and BoxCharType to TableGen type definition.Feb 10 2021, 12:46 PM
mehdi_amini added inline comments.Feb 10 2021, 1:52 PM
flang/include/flang/Optimizer/Dialect/FIRTypes.td
51

Is this a matter of updating the TableGen backend to forward declare all classes before emitting the content?
I had to do this with operations a while ago for a similar reason.

116

you have a non negligible amount of C++ here, consider replacing this with simple calls to static functions implemented in the cpp file. TableGen files don't have all the C++ IDE features...

rriddle added inline comments.Feb 10 2021, 1:56 PM
flang/include/flang/Optimizer/Dialect/FIRTypes.td
63

Can you move the printer/parser to C++ instead? You just have to remove these fields and then you can provide your own definition for CharacterType::parse and CharacterType::print.

clementval marked 3 inline comments as done.Feb 10 2021, 5:12 PM
clementval added inline comments.
flang/include/flang/Optimizer/Dialect/FIRTypes.td
51

Moving the definitions in the .cpp files does the trick.

clementval marked an inline comment as done.

Address review comment + rebase

Keep fir_ prefix

schweitz accepted this revision.Feb 12 2021, 3:57 PM
This revision is now accepted and ready to land.Feb 12 2021, 3:57 PM