[PATCH] D78835: [flang] Upstream recent work on FIR to llvm-project.

[River Riddle via Phabricator via llvm-commits]

rriddle added inline comments.


================
Comment at: flang/lib/Optimizer/Dialect/FIROps.cpp:593
+      "internal", "common", "weak"};
+  if (llvm::any_of(validNames,
+                   [&](const char *name) { return linkage == name; }))
----------------
`return success(llvm::is_contained(validNames, linkage));` ?


================
Comment at: flang/lib/Optimizer/Dialect/FIROps.cpp:617
+  bodyRegion->front().addArgument(iterate.getType());
+  for (auto v : iterArgs)
+    bodyRegion->front().addArgument(v.getType());
----------------
`bodyRegion->front().addArguments(iterArgs.getTypes());`?


================
Comment at: flang/lib/Optimizer/Dialect/FIROps.cpp:805
   bodyRegion->front().addArgument(builder->getIndexType());
+  for (auto v : iterArgs)
+    bodyRegion->front().addArgument(v.getType());
----------------
Same here.


================
Comment at: flang/lib/Optimizer/Dialect/FIROps.cpp:994
+  llvm::SmallVector<mlir::OpAsmParser::OperandType, 4> operands;
+  llvm::SmallVector<mlir::Type, 4> types;
+  llvm::SMLoc loc = parser.getCurrentLocation();
----------------
This could be written declaratively like:

```
let assemblyFormat = "($results^ `:` type($results))? attr-dict";
```

using optional groups:
https://mlir.llvm.org/docs/OpDefinitions/#optional-groups


================
Comment at: flang/lib/Optimizer/Dialect/FIROps.cpp:1032
+static unsigned denseElementsSize(mlir::DenseIntElementsAttr attr) {
+  unsigned count = 0;
+  for (auto x : attr)
----------------
This seems weird. `attr.getNumElements()`?


================
Comment at: flang/lib/Optimizer/Dialect/FIROps.cpp:1464
+mlir::LogicalResult
+fir::WhereOp::fold(llvm::ArrayRef<mlir::Attribute> opnds,
+                   llvm::SmallVectorImpl<mlir::OpFoldResult> &results) {
----------------
This doesn't seem to do anything? Seems WhereOp is already marked as RecursiveSideEffects which would cause it be deleted in cases where the bodies were empty.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78835