[PATCH] D114148: [fir] Add fircg.ext_embox conversion
Kiran Chandramohan via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Nov 18 05:22:41 PST 2021
kiranchandramohan added a comment.
Minor comments and request for two more tests.
================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:1405-1421
+ inline mlir::Value insertLowerBound(mlir::ConversionPatternRewriter &rewriter,
+ mlir::Location loc, mlir::Value dest,
+ unsigned dim, mlir::Value lb) const {
+ return insertField(rewriter, loc, dest, {7, dim, 0}, lb);
+ }
+
+ inline mlir::Value insertExtent(mlir::ConversionPatternRewriter &rewriter,
----------------
Nit: add the name for the pos 7.
================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:1525
+ } else {
+ gepOperands.push_back(lowerBound);
+ }
----------------
Nit: else can be removed by moving the lowerBound pushing outside.
================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:1551
+ mlir::Value step, mlir::Value zero, mlir::Type type) {
+ // auto type = ub.getType();
+ mlir::Value extent = rewriter.create<mlir::LLVM::SubOp>(loc, type, ub, lb);
----------------
Nit: remove commented code.
================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:1633
+ if (fir::hasDynamicSize(seqEleTy)) {
+ if (fir::isa_char(seqEleTy)) {
+ assert(xbox.lenParams().size() == 1);
----------------
Is this case covered in the test? (the char has a dynamic size)
================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:1651
+ mlir::Value stepExpr;
+ if (hasSubcomp) {
+ // We have a subcomponent. The step value needs to be the number of
----------------
I think we need a test for derived type as well.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114148/new/
https://reviews.llvm.org/D114148
More information about the llvm-commits
mailing list