[PATCH] D113551: [fir] Add fir.box_rank, fir.box_addr, fir.box_dims and fir.box_elesize conversion

[Andrzej Warzynski via Phabricator via llvm-commits]

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

Thanks for the updates,

Makes sense. The remaining comments are mostly nits, so I'm accepting as is.



================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:54
+
+  /// Method to construct code sequence to get the rank from a box.
+  mlir::Value getRankFromBox(mlir::Location loc, mlir::Value box,
----------------
[nit] It's clear that it's a method. `Construct` is the key information here.


================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:73
+                 mlir::ConversionPatternRewriter &rewriter) const {
+    mlir::LLVM::ConstantOp c0 = genConstantOffset(loc, rewriter, 0);
+    mlir::LLVM::ConstantOp cDims =
----------------
[nit]


================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:111-113
+    mlir::LLVM::ConstantOp c0 = genConstantOffset(loc, rewriter, 0);
+    mlir::LLVM::ConstantOp c1 =
+        genConstantOffset(loc, rewriter, kElemLenPosInBox);
----------------
`c0` is the offset to get the relevant struct (i.e. to get the base address), and `c1` is the offset within the struct (i.e. to get the element address), right?


================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:123
+                           mlir::Value base, ARGS... args) const {
+    SmallVector<mlir::Value> cv{args...};
+    return rewriter.create<mlir::LLVM::GEPOp>(loc, ty, base, cv);
----------------
What's `cv`?


================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:228
+/// the box.
+/// The rank is in 3rd position in the box.
+struct BoxRankOpConversion : public FIROpConversion<fir::BoxRankOp> {
----------------
DELETEME

This is an implementation detail from `getRankFromBox`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113551