[PATCH] D95399: [flang][fir] Upstream FIR dialect changes.

[Eric Schweitz via Phabricator via llvm-commits]

schweitz added inline comments.


================
Comment at: flang/lib/Optimizer/Dialect/FIROps.cpp:135
+    }
+  return result;
+}
----------------
mehdi_amini wrote:
> This method is a bit strange: it returns `result` here but I don't see it ever modified?
> 
> If you only ever returns ranges of Value held by an operation, then `ValueRange` seems appropriate instead of `std::vector<Value>`
Good points. We'll see if we can tweak this one.


================
Comment at: flang/lib/Optimizer/Dialect/FIROps.cpp:467
     if (fir::GlobalOp::verifyValidLinkage(linkage))
-      return failure();
+      return mlir::failure();
     mlir::StringAttr linkAttr = builder.getStringAttr(linkage);
----------------
mehdi_amini wrote:
> It isn't clear why this is needed?
Not sure what "this" refers to, but I assume you mean the check/test. This is older code--it may have been here since before MLIR was merged into the mono repo--that predates some of the improvements made to the autogenerated verification code in the last year or so. It may not longer be needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95399