-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[NFC][flang] Introduce FortranObjectViewOpInterface. #166841
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,7 +39,8 @@ def hlfir_DeclareOp | |
| : hlfir_Op<"declare", [AttrSizedOperandSegments, | ||
| MemoryEffects<[MemAlloc<DebuggingResource>]>, | ||
| DeclareOpInterfaceMethods< | ||
| fir_FortranVariableStorageOpInterface>]> { | ||
| fir_FortranVariableStorageOpInterface>, | ||
| fir_FortranObjectViewOpInterface]> { | ||
| let summary = "declare a variable and produce an SSA value that can be used as a variable in HLFIR operations"; | ||
|
|
||
| let description = [{ | ||
|
|
@@ -140,6 +141,10 @@ def hlfir_DeclareOp | |
| /// Given a FIR memory type, and information about non default lower | ||
| /// bounds, get the related HLFIR variable type. | ||
| static mlir::Type getHLFIRVariableType(mlir::Type type, bool hasLowerBounds); | ||
|
|
||
| // FortranObjectViewOpInterface methods: | ||
| mlir::Value getViewSource(mlir::OpResult) { return getMemref(); } | ||
| std::optional<std::int64_t> getViewOffset(mlir::OpResult) { return 0; } | ||
| }]; | ||
|
|
||
| let hasVerifier = 1; | ||
|
|
@@ -213,8 +218,11 @@ def fir_AssignOp : hlfir_Op<"assign", [DeclareOpInterfaceMethods<MemoryEffectsOp | |
| let hasVerifier = 1; | ||
| } | ||
|
|
||
| def hlfir_DesignateOp : hlfir_Op<"designate", [AttrSizedOperandSegments, | ||
| DeclareOpInterfaceMethods<fir_FortranVariableOpInterface>, NoMemoryEffect]> { | ||
| def hlfir_DesignateOp | ||
| : hlfir_Op<"designate", | ||
| [AttrSizedOperandSegments, | ||
| DeclareOpInterfaceMethods<fir_FortranVariableOpInterface>, | ||
| NoMemoryEffect, fir_FortranObjectViewOpInterface]> { | ||
| let summary = "Designate a Fortran variable"; | ||
|
|
||
| let description = [{ | ||
|
|
@@ -278,6 +286,9 @@ def hlfir_DesignateOp : hlfir_Op<"designate", [AttrSizedOperandSegments, | |
| void setFortranAttrs(fir::FortranVariableFlagsEnum flags) { | ||
| this->setFortranAttrs(std::optional<fir::FortranVariableFlagsEnum>(flags)); | ||
| } | ||
| // FortranObjectViewOpInterface methods: | ||
| mlir::Value getViewSource(mlir::OpResult) { return getMemref(); } | ||
| std::optional<std::int64_t> getViewOffset(mlir::OpResult); | ||
| }]; | ||
|
|
||
| let builders = [ | ||
|
|
@@ -938,8 +949,9 @@ def hlfir_EndAssociateOp : hlfir_Op<"end_associate", [MemoryEffects<[MemFree]>]> | |
| let hasVerifier = 1; | ||
| } | ||
|
|
||
| def hlfir_AsExprOp : hlfir_Op<"as_expr", | ||
| [DeclareOpInterfaceMethods<MemoryEffectsOpInterface>]> { | ||
| def hlfir_AsExprOp | ||
| : hlfir_Op<"as_expr", [DeclareOpInterfaceMethods<MemoryEffectsOpInterface>, | ||
| fir_FortranObjectViewOpInterface]> { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this needed? I do not think AsExpr is a view op, It result should be regarded as a value/tensor with no memory connection to the input. When it has not been eliminated, it may end-up being implemented as a copy or as use the original storage if it is proven safe. I think as_expr may have been given some handling in aliasing so that it could have an easier code generation. I expect this was done so that hlfir.assign optimization would not optimize hlfir.asssign of hlfir.elemental using hlfir.as_expr in case the as_expr source overlaps with the LHS assuming that as_expr codegen could end up using the source. But I feel the problem should be seen in the other direction, and it is up to the as_expr codegen to prove that it can safely replace the copy by its source. If possible, I would prefer keeping the ad-hoc handling in FIR aliasing so that we can try to fix this instead of enshrining the assumption that as_expr result is based on its source in the operation definition. |
||
| let summary = "Take the value of an array, character or derived variable"; | ||
|
|
||
| let description = [{ | ||
|
|
@@ -961,8 +973,11 @@ def hlfir_AsExprOp : hlfir_Op<"as_expr", | |
| let results = (outs hlfir_ExprType); | ||
|
|
||
| let extraClassDeclaration = [{ | ||
| // Is this a "move" ? | ||
| bool isMove() { return getMustFree() != mlir::Value{}; } | ||
| // Is this a "move" ? | ||
| bool isMove() { return getMustFree() != mlir::Value{}; } | ||
| // FortranObjectViewOpInterface methods: | ||
| mlir::Value getViewSource(mlir::OpResult) { return getVar(); } | ||
| std::optional<std::int64_t> getViewOffset(mlir::OpResult) { return 0; } | ||
| }]; | ||
|
|
||
| let assemblyFormat = [{ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this correct for common block variables?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question! Yes, it should be valid.
For example:
Basically,
[hl]fir.declaredoes not apply any offset to its operand on its own. The offset is applied to the operands of[hl]fir.declarebyfir.coordinate_of. It is also the case for EQUIVALENCE variables.