[PATCH] D111337: [fir] Add array value copy pass

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added a comment.

That is.... large. I can't really review this.

I'm curious about what a test-coverage would show on this? What is the proportion of uncovered code here?



================
Comment at: flang/include/flang/Optimizer/Builder/DoLoopHelper.h:28
+  /// Type of a callback to generate the loop body.
+  using BodyGenerator = std::function<void(fir::FirOpBuilder &, mlir::Value)>;
+
----------------
If you're not storing it, can you use `llvm::function_ref` instead?
(and pass by value)


================
Comment at: flang/lib/Optimizer/Builder/FIRBuilder.cpp:30
+                                      "name"),
+                       llvm::cl::init(32));
+
----------------
I'm wary of global options in general (Can we build the optimizer with `-Werror=global-constructors` by the way?), what is this used for? Can you find an alternative (like store it on the dialect for example)?


================
Comment at: flang/lib/Optimizer/Builder/FIRBuilder.cpp:46
+                                                llvm::StringRef name) {
+  return modOp.lookupSymbol<fir::GlobalOp>(name);
+}
----------------
Do these one-line functions carry their weight? 
Seems like just a renaming that kind of obfuscate what's happening (I'd have to jump through more hoops in the IDE to figure it out).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111337