[PATCH] D113756: [fir] Add fir.embox conversion
Valentin Clement via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 15 04:12:32 PST 2021
clementval marked 14 inline comments as done.
clementval added inline comments.
================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:1062
+ ? mlir::cast<mlir::LLVM::LLVMFuncOp>(op)
+ : op->getParentOfType<mlir::LLVM::LLVMFuncOp>();
+ rewriter.setInsertionPointToStart(&func.front());
----------------
kiranchandramohan wrote:
> rovka wrote:
> > Do we really need op, or could we just replace all of this with
> >
> > ```
> > auto func = thisBlock->getParentOfType<mlir::LLVM::LLVMFuncOp>()
> > ```
> > ?
> I think such a function does not exist.
@kiranchandramohan is right. Block does not have such function.
================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:1323
+ mlir::ConversionPatternRewriter &rewriter) const override {
+ // There should be no dims on this embox op
+ assert(!embox.getShape());
----------------
awarzynski wrote:
> 1. Why?
> 2. Could you "inject" this comment into the `assert`?
If I'm not wrong embox with shape are first lowered to xembox op and then converted to LLVM IR.
2. done
================
Comment at: flang/test/Fir/convert-to-llvm.fir:1218
+// CHECK: %[[ELEM_SIZE:.*]] = llvm.mlir.constant(4 : i32) : i32
+// CHECK: %[[TYPE_CODE:.*]] = llvm.mlir.constant(9 : i32) : i32
+// CHECK: %[[I64_ELEM_SIZE:.*]] = llvm.sext %3 : i32 to i64
----------------
rovka wrote:
> awarzynski wrote:
> > Where is `9` coming from? I couldn't find it.
> https://github.com/llvm/llvm-project/blob/4119da2f7c5fd21c03f6d50aa1d2af3527a9e90f/flang/include/flang/ISO_Fortran_binding.h#L56 ?
That's correct.
================
Comment at: flang/test/Fir/convert-to-llvm.fir:1219
+// CHECK: %[[TYPE_CODE:.*]] = llvm.mlir.constant(9 : i32) : i32
+// CHECK: %[[I64_ELEM_SIZE:.*]] = llvm.sext %3 : i32 to i64
+// CHECK: %[[DESC0:.*]] = llvm.insertvalue %[[I64_ELEM_SIZE]], %[[DESC]][1 : i32] : !llvm.struct<(ptr<array<100 x i32>>, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}})>
----------------
awarzynski wrote:
>
Thanks for catching this.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113756/new/
https://reviews.llvm.org/D113756
More information about the llvm-commits
mailing list