Lower initial data target #688
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
schweitzpgi
approved these changes
Mar 18, 2021
| type = ty; | ||
| if (auto seqType = type.dyn_cast<fir::SequenceType>()) | ||
| type = seqType.getEleTy(); | ||
| return type; |
There was a problem hiding this comment.
I'm surprised this isn't already implemented someplace. Seems to pop up often.
There was a problem hiding this comment.
Right, we had a discussion a while ago about having this in FIRType.h. For types that wraps character/record, we might want quick access on the character/record type to take decisions based on the length parameters. The conclusion at the time was that it was too custom (so there is something very similar in CharacterExpr.cpp).
Would you be in favor of moving this in FIRType.h ?
There was a problem hiding this comment.
Seems like the best place for it, if it can reduce some replication.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Create a function
genInitialDataTargetto create afir.boxfor a pointer initial data target. IndefineGlobalanddefineCommon, use it if the global (or common member) is a pointer with an initial data target.This is not added to
defineGlobalAggregateStorebecauseC8106prevents pointer/derived type with pointer component to be used in equivalences.The fact that the initial data target cannot be in an equivalence (C8108 + C765) also simplifies the logic (there is no need to try instantiating aggregate stores inside global op initializer).
The main Fortran difficulty was that the initial data target can be in common block, and so can the pointer. This is covered by the
instantiateVar(var)call on the initial data target that will declare the common block if needed (as this call it does in normal context).Most of the complexity of
genInitialDataTargetstems from the fact that it is not possible to usefir.reboxin a global initializer (which we normally used in some pointer assignment case). I tried making this possible, but it was driving too much weird changes infir.reboxcodegen to support both an in memory and a struct descriptor argument. So I went for an ad-hoc handling of this corner case.The last test cases
block data snakeandblock data tiedinpointer-initial-target-2.f90lowers fine to FIR and LLVM dialect, but crashes the LLVM dialect to LLVM IR pass. I am working on a simple fix for this, the issue turned out to be that the translation passes declare global op and define their body in the same pass, which is an issue if they are forward reference from an initializer to a globalOp not yet declared in LLVM IR.Derived type with length parameters are left TODOs. I can't prepare much here without more general work on them.