[PATCH] D114709: [Flang] Upstream conversion of the XRebox Op

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan added inline comments.


================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:1709
+private:
+  inline mlir::Type getVoidPtrType(mlir::MLIRContext *context) const {
+    return mlir::LLVM::LLVMPointerType::get(mlir::IntegerType::get(context, 8));
----------------
I had to bring this function inside the pattern since the compiler was complaining that it is not able to find this function. But it seems to work fine in the fir-dev branch. https://github.com/flang-compiler/f18-llvm-project/blob/b111a42388dd1516ff8beb126cf53cd573737b4a/flang/lib/Optimizer/CodeGen/CodeGen.cpp#L92

Any ideas?


================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:1787
+         ++i, ++strideOps, ++shiftOps, sliceOps += 3) {
+      auto sliceLb = integerCast(loc, rewriter, idxTy, operands[sliceOps]);
+      auto inputStride = *strideOps; // already idxTy
----------------
@jeanPerier  Here and in a few other places we were using the original operands and not the converted operands. This caused some issue when I cherry-picked the patch to llvm-project/flang. In discourse it was pointed out that this is because we are not using the converted operands (https://llvm.discourse.group/t/issue-in-conversion-from-index-to-i64/4853). Is there any reason we were using the original operands? 
For getting the patch to work I had to switch to the converted operands.


================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:1827
+             mlir::ConversionPatternRewriter &rewriter) const {
+    mlir::ValueRange reboxShifts{operands.begin() + rebox.shiftOffset(),
+                                 operands.begin() + rebox.shiftOffset() +
----------------
Is there a better way to do this valuerange creation?


================
Comment at: flang/test/Fir/convert-to-llvm.fir:1596
+
+// Check `fircg.ext_rebox` conversion to LLVM IR dialect
+
----------------
More tests are required for derived types and character.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114709