[PATCH] D113089: [fir] Add fir.select and fir.select_rank FIR to LLVM IR conversion patterns

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini accepted this revision.
mehdi_amini added inline comments.
This revision is now accepted and ready to land.


================
Comment at: flang/include/flang/Optimizer/Dialect/FIROps.td:487
     // The selector is the value being tested to determine the destination
     mlir::Value getSelector() { return selector(); }
     mlir::Value getSelector(llvm::ArrayRef<mlir::Value> operands) {
----------------
If this is just about having a `get` prepended, there is an option in ODS to do this universally (since https://reviews.llvm.org/D111033 ).
I don't think it is scalable to just alias manually every possible accessor.

You can just add `let emitAccessorPrefix = kEmitAccessorPrefix_Both;` in the Dialect ODS definition to get both forms generated.




================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:156
+                           mlir::ConversionPatternRewriter &rewriter) {
+  auto conds = select.getNumConditions();
+  auto caseAttr =
----------------
Can we just use int? (or `int64_t`)
(The code with the decltype seems just really unusual to me)



================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:158
+  auto caseAttr =
+      select->template getAttrOfType<mlir::ArrayAttr>(OP::getCasesAttr());
+  auto cases = caseAttr.getValue();
----------------
Seems like a missing accessor on `fir_SwitchTerminatorOp` in the ODS file?


================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:160
+  auto cases = caseAttr.getValue();
+  auto selector = select.getSelector(adaptor.getOperands());
+  auto loc = select.getLoc();
----------------



================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:173
+    auto destOps = select.getSuccessorOperands(adaptor.getOperands(), t);
+    auto &attr = cases[t];
+    if (auto intAttr = attr.template dyn_cast<mlir::IntegerAttr>()) {
----------------
Attributes are value types


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113089