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

[Kiran Chandramohan via Phabricator via flang-commits]

kiranchandramohan added a comment.

Thanks for describing the changes in detail and for putting them up for review before the code changes. I have a few questions. None of them are blocking, so feel free to push forward if necessary.

It will be good if you can mention in the summary that this patch introduces `hlfir.declare` operation. It will be great if we can have a table/summary describing what information is present in `hlfir.declare` vs `fir.declare`.



================
Comment at: flang/docs/HighLevelFIR.md:76
+fir.box<T>, fir.boxchar or fir.ref<fir.box<T>>). Not all memory types will be
+allowed for a variable: it should allow retrieving all the shape, type
+parameters, and dynamic type information without requiring to look-up for any
----------------
Is this not allowed for an hlfir variable or for fir variables too?


================
Comment at: flang/docs/HighLevelFIR.md:95
+hlfir.declare, but not to assume that it must be visible.  For instance, simple
+contiguity of fir.box can be deduced in certain case from the hlfir.declare,
+and if the hlfir.declare cannot be found, the transformation passe will have to
----------------
Would this information be available in fir.declare as well? Or should any pass requiring such information operate at the hlfir level?


================
Comment at: flang/docs/HighLevelFIR.md:283-284
 
-%var will have the same type as %base. When no debug info is generated, the
-operation can be replaced by %base when lowering to LLVM.
+%var#0 will have a FIR memory type that is allowed for HLFIR variables. %var#1
+will have the same type as %base, it is intended to be used when lowering HLFIR
+to FIR in order to avoid creating unnecessary fir.box (that would become
----------------
Is it not possible to create `%var#1` from `%var#0`? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137634