[flang-commits] [flang] 3ce7e4b - [flang] fix fir.array_coor of fir.box with component references

[Jean Perier via flang-commits]

Author: Jean Perier
Date: 2023-04-14T08:47:50+02:00
New Revision: 3ce7e4b28d6ef8bdaf4ed22bf0337c70ccee8149

URL: https://github.com/llvm/llvm-project/commit/3ce7e4b28d6ef8bdaf4ed22bf0337c70ccee8149
DIFF: https://github.com/llvm/llvm-project/commit/3ce7e4b28d6ef8bdaf4ed22bf0337c70ccee8149.diff

LOG: [flang] fix fir.array_coor of fir.box with component references

When dealing with "derived_array(j)%component" where derived_array
is not a contiguous array, but for which we know the extent, lowering
generates a fir.array_coor op on a !fir.box<!fir.array<cst x T>> with
a fir.slice containing "j" in the component path.

Codegen first computes "derived_array(j)" address using the byte
strides inside the descriptor, and then computes the offset of "j"
from that address with a second GEP.
The type of the address in that second GEP matters since "j" is passed
in the GEP via an index indicating its component position in the type.

The code was using the LLVM type of "derived_array" instead of
"derived_array(j)".
In general, with fir.box, the extent ("cst" above) is unknown and those
types match. But if the extent of "derived_array" is a compile time
constant, its LLVM type will be [cst x T] instead of T*, and the produced
GEP will compute the address of the nth T instead of the nth component
inside T leading to undefined behaviors.

Fix this by computing the element type for the second GEP.

Differential Revision: https://reviews.llvm.org/D148226

Added: 
    

Modified: 
    flang/lib/Optimizer/CodeGen/CodeGen.cpp
    flang/test/Fir/array-coor.fir

Removed: 
    


################################################################################
diff  --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
index bbec5fc9d3493..0bdc2049334bc 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
@@ -2448,7 +2448,16 @@ struct XArrayCoorOpConversion
         rewriter.replaceOpWithNewOp<mlir::LLVM::BitcastOp>(coor, ty, addr);
         return mlir::success();
       }
-      auto casted = rewriter.create<mlir::LLVM::BitcastOp>(loc, baseTy, addr);
+      // Cast the element address from void* to the derived type so that the
+      // derived type members can be addresses via a GEP using the index of
+      // components.
+      mlir::Type elementType =
+          baseTy.cast<mlir::LLVM::LLVMPointerType>().getElementType();
+      while (auto arrayTy = elementType.dyn_cast<mlir::LLVM::LLVMArrayType>())
+        elementType = arrayTy.getElementType();
+      mlir::Type elementPtrType = mlir::LLVM::LLVMPointerType::get(elementType);
+      auto casted =
+          rewriter.create<mlir::LLVM::BitcastOp>(loc, elementPtrType, addr);
       args.clear();
       args.push_back(0);
       if (!coor.getLenParams().empty()) {

diff  --git a/flang/test/Fir/array-coor.fir b/flang/test/Fir/array-coor.fir
index 2177f95303697..738acd7dd91fd 100644
--- a/flang/test/Fir/array-coor.fir
+++ b/flang/test/Fir/array-coor.fir
@@ -20,3 +20,26 @@ func.func @array_coor_box_value(%29 : !fir.box<!fir.array<2xf64>>,
 // CHECK: %[[t11:.*]] = getelementptr i8, ptr %[[t10]], i64 %[[t8]]
 // CHECK: %[[t12:.*]] = load double, ptr %[[t11]]
 // CHECK: ret double %[[t12]]
+
+func.func @test_array_coor_box_component_slice(%arg0: !fir.box<!fir.array<2x!fir.type<t{i:i32,j:i32}>>>) {
+  %c2 = arith.constant 2 : index
+  %c1 = arith.constant 1 : index
+  %0 = fir.field_index j, !fir.type<t{i:i32,j:i32}>
+  %1 = fir.slice %c1, %c2, %c1 path %0 : (index, index, index, !fir.field) -> !fir.slice<1>
+  %2 = fir.array_coor %arg0 [%1] %c2 : (!fir.box<!fir.array<2x!fir.type<t{i:i32,j:i32}>>>, !fir.slice<1>, index) -> !fir.ref<i32>
+  fir.call @take_int(%2) : (!fir.ref<i32>) -> ()
+  return
+}
+func.func private @take_int(%arg0: !fir.ref<i32>) -> ()
+
+// CHECK-LABEL: define void @test_array_coor_box_component_slice(
+// CHECK-SAME: ptr %[[VAL_0:.*]])
+// CHECK:   %[[VAL_1:.*]] = getelementptr { ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]], ptr, [1 x i64] }, ptr %[[VAL_0]], i32 0, i32 7, i32 0, i32 2
+// CHECK:   %[[VAL_2:.*]] = load i64, ptr %[[VAL_1]]
+// CHECK:   %[[VAL_3:.*]] = mul i64 1, %[[VAL_2]]
+// CHECK:   %[[VAL_4:.*]] = add i64 %[[VAL_3]], 0
+// CHECK:   %[[VAL_5:.*]] = getelementptr { ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]], ptr, [1 x i64] }, ptr %[[VAL_0]], i32 0, i32 0
+// CHECK:   %[[VAL_6:.*]] = load ptr, ptr %[[VAL_5]]
+// CHECK:   %[[VAL_7:.*]] = getelementptr i8, ptr %[[VAL_6]], i64 %[[VAL_4]]
+// CHECK:   %[[VAL_8:.*]] = getelementptr %t, ptr %[[VAL_7]], i32 0, i32 1
+// CHECK:   call void @take_int(ptr %[[VAL_8]])