[flang-commits] [PATCH] D142043: [flang][hlfir] Enable lowering and passing of allocatables and pointers.

[Pete Steinfeld via Phabricator via flang-commits]

PeteSteinfeld accepted this revision.
PeteSteinfeld added a comment.
This revision is now accepted and ready to land.

Aside from some nits and questions, all builds and tests correctly and looks good.



================
Comment at: flang/include/flang/Optimizer/Builder/HLFIRTools.h:137
 
+  // Is this entity know to be contiguous at compile time?
+  // Note that when this return false, the entity may still
----------------
"know" should be "known"


================
Comment at: flang/include/flang/Optimizer/Dialect/FortranVariableInterface.td:125
 
+    /// Does this variable has the Fortran CONTIGUOUS attribute?
+    /// Note that not having this attribute does not imply the
----------------
"has" should be "have"


================
Comment at: flang/lib/Optimizer/Builder/HLFIRTools.cpp:74-78
+static void
+genExtentsAndLboundFromBox(mlir::Location loc, fir::FirOpBuilder &builder,
+                           hlfir::Entity boxEntity,
+                           llvm::SmallVectorImpl<mlir::Value> *lbounds,
+                           llvm::SmallVectorImpl<mlir::Value> *extents) {
----------------
This interface seems a little off to me.  In the name "Extents" is plural, while "Lbound" is singular.  Also, in every call, the "lbounds" argument is non-null, but the code allows for it to be null.  Would it be better to assert that it's non-null?


================
Comment at: flang/lib/Optimizer/Builder/HLFIRTools.cpp:419
+      assert(variable.getType().isa<fir::BaseBoxType>() &&
+             "array variable with dynamic extent must be boxes");
+      mlir::Value dim =
----------------
Is "boxes" supposed to be "boxed"?


================
Comment at: flang/lib/Optimizer/Builder/HLFIRTools.cpp:698
+      !variable.getIfVariableInterface()) {
+    // This special case avoid generating two generating to sets of identical
+    // fir.box_dim to get both the lower bounds and extents.
----------------
"avoid" should be "avoids".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142043