[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