[flang-commits] [PATCH] D137634: [flang][RFC] Do not rely on attributes to tag HLFIR variable uses

[Jean Perier via Phabricator via flang-commits]

jeanPerier marked 3 inline comments as done.
jeanPerier added a comment.

Thanks for the rewiew

In D137634#3918988 <https://reviews.llvm.org/D137634#3918988>, @kiranchandramohan wrote:

> While for the near term, hlfir.declare will simplify the OpenMP privatisation clause handling but when we bring in the privatisation clauses to the OpenMP dialect, the dialect will have to live with multiple variable types (hlfir.declare, fir.declare, fir.alloca, llvm.alloca etc).

They are three things that could maybe help here:

1. hlfir.declare and fir.declare share a common FortranVariableOpInterface that could be relied on
2. hlfir.declare and fir.declare will probably not co-exists (so depending on when privatization occur, only one or the other would matter)
3. the declare ops are not really doing anything with memory, they are just tagging it, and binding info to it, so maybe it is OK to only care about the alloc ops as far as privatization is concerned.



================
Comment at: flang/docs/HighLevelFIR.md:114
 
+The hlfir.declare will have a sibling fir.declare operation in FIR that will
+allow keeping variable information until debug info is generated. The main
----------------
kiranchandramohan wrote:
> Will these two co-exist? Or will `fir.declare`  be generated from `hlfir.declare` ? 
It is possible for the two to coexist, but the plan is for fir.declare to be generated from hlfir.declare while converting HLFIR to FIR.


================
Comment at: flang/docs/HighLevelFIR.md:125
 Note that Fortran variables are not necessarily named objects, they can also be
-the result of function references returning POINTERs. fir.declare will also
+the result of function references returning POINTERs. hfir.declare will also
 accept such variables to be described in the IR (a unique name will be built
----------------
kiranchandramohan wrote:
> fir.declare or hlfir.declare?
hlfir.declare, thanks


================
Comment at: flang/docs/HighLevelFIR.md:1337
+
+Instead of restricting what memory types an HLFIR variable it was considered to
+force the defining operation of HLFIR variable SSA values to always be
----------------
kiranchandramohan wrote:
> Something is missing in this sentence.
Repharsed to "Instead of restricting the memory types an HLFIR variable can have, it was"


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137634/new/

https://reviews.llvm.org/D137634