[PATCH] D96987: [flang][fir][NFC] Move remaining types to TableGen type definition

Valentin Clement via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 19 19:49:43 PST 2021


clementval added a comment.

In D96987#2576120 <https://reviews.llvm.org/D96987#2576120>, @rriddle wrote:

> Do the parsers/printers here have any test coverage?

Round trip tests in `flang/test/Fir/fir-types.fir`. I'm gonna upstream some "invalid" tests in a follow up patch as well to check the diagnostics.



================
Comment at: flang/include/flang/Optimizer/Dialect/FIRTypes.td:425
+    mlir::Type getElementType() const {
+      return getEleTy();
+    }
----------------
rriddle wrote:
> Renaming the parameter to `elementType` would remove the need for this.
It's not needed anymore so I just removed it. 


================
Comment at: flang/lib/Optimizer/Dialect/FIRType.cpp:300
+  return eleTy.isa<BoxType>() || eleTy.isa<BoxCharType>() ||
+         eleTy.isa<BoxProcType>() || eleTy.isa<ShapeType>() ||
+         eleTy.isa<ShapeShiftType>() || eleTy.isa<SliceType>() ||
----------------
rriddle wrote:
> You should be able to do: `isa<BoxType, BoxCharType, ...>()`
Right. It's not new code so I didn't double checked it. Works fine with your suggestion. Thx!


================
Comment at: flang/lib/Optimizer/Dialect/FIRType.cpp:768
   auto shape = getShape();
   for (unsigned i{rows}, size{dim}; i < size; ++i)
     if (shape[i] != getUnknownExtent())
----------------
rriddle wrote:
> `i = rows`? The initialization here looks really weird,
It's also a code that was just moved around. I updated the initialization in the new patch version. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96987



More information about the llvm-commits mailing list