From d3bc3a0400414a5c71cc029fa42272d647e3b81b Mon Sep 17 00:00:00 2001 From: Jean Perier Date: Fri, 25 Mar 2022 09:02:48 +0100 Subject: [PATCH] [flang][codegen] ensure descriptor lower bounds are LBOUND compliant Follow-up of https://reviews.llvm.org/D121488 to ensure all descriptors created inline complies with LBOUND requirement that the lower bound is `1` when the related dimension extent is zero. Both fir.xrebox and fir.xembox codegen is updated to enforce this constraint. Also upstream the "normalized lower bound" attribute that was added in fir-dev since embox codegen was upstreamed, it is conflicting with this patch otherwise. Differential Revision: https://reviews.llvm.org/D122419 --- .../include/flang/Optimizer/Dialect/FIROps.h | 4 ++ flang/lib/Optimizer/CodeGen/CodeGen.cpp | 46 +++++++++++++++---- flang/test/Fir/convert-to-llvm.fir | 14 ++++-- 3 files changed, 50 insertions(+), 14 deletions(-) diff --git a/flang/include/flang/Optimizer/Dialect/FIROps.h b/flang/include/flang/Optimizer/Dialect/FIROps.h index 68202a182889b..4e2b490999e90 100644 --- a/flang/include/flang/Optimizer/Dialect/FIROps.h +++ b/flang/include/flang/Optimizer/Dialect/FIROps.h @@ -40,6 +40,10 @@ static constexpr llvm::StringRef getAdaptToByRefAttrName() { return "adapt.valuebyref"; } +static constexpr llvm::StringRef getNormalizedLowerBoundAttrName() { + return "normalized.lb"; +} + } // namespace fir #define GET_OP_CLASSES diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp index 5f65eda63ff8c..affed344577df 100644 --- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp +++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp @@ -1529,20 +1529,30 @@ struct XEmboxOpConversion : public EmboxCommonConversion { } } if (!skipNext) { - // store lower bound (normally 0) + if (hasSlice) + extent = computeTripletExtent(rewriter, loc, operands[sliceOffset], + operands[sliceOffset + 1], + operands[sliceOffset + 2], zero, i64Ty); + // store lower bound (normally 0) for BIND(C) interoperability. mlir::Value lb = zero; - if (eleTy.isa() || eleTy.isa()) { + const bool isaPointerOrAllocatable = + eleTy.isa() || eleTy.isa(); + // Lower bound is defaults to 1 for POINTER, ALLOCATABLE, and + // denormalized descriptors. + if (isaPointerOrAllocatable || !normalizedLowerBound(xbox)) { lb = one; - if (hasShift) + // If there is a shifted origin and this is not a normalized + // descriptor then use the value from the shift op as the lower bound. + if (hasShift) { lb = operands[shiftOffset]; + auto extentIsEmpty = rewriter.create( + loc, mlir::LLVM::ICmpPredicate::eq, extent, zero); + lb = rewriter.create(loc, extentIsEmpty, one, + lb); + } } dest = insertLowerBound(rewriter, loc, dest, descIdx, lb); - // store extent - if (hasSlice) - extent = computeTripletExtent(rewriter, loc, operands[sliceOffset], - operands[sliceOffset + 1], - operands[sliceOffset + 2], zero, i64Ty); dest = insertExtent(rewriter, loc, dest, descIdx, extent); // store step (scaled by shaped extent) @@ -1597,6 +1607,13 @@ struct XEmboxOpConversion : public EmboxCommonConversion { rewriter.replaceOp(xbox, result); return success(); } + + /// Return true if `xbox` has a normalized lower bounds attribute. A box value + /// that is neither a POINTER nor an ALLOCATABLE should be normalized to a + /// zero origin lower bound for interoperability with BIND(C). + inline static bool normalizedLowerBound(fir::cg::XEmboxOp xbox) { + return xbox->hasAttr(fir::getNormalizedLowerBoundAttrName()); + } }; /// Create a new box given a box reference. @@ -1662,12 +1679,21 @@ struct XReboxOpConversion : public EmboxCommonConversion { mlir::ValueRange strides, mlir::ConversionPatternRewriter &rewriter) const { mlir::Location loc = rebox.getLoc(); + mlir::Value zero = + genConstantIndex(loc, lowerTy().indexType(), rewriter, 0); mlir::Value one = genConstantIndex(loc, lowerTy().indexType(), rewriter, 1); for (auto iter : llvm::enumerate(llvm::zip(extents, strides))) { + mlir::Value extent = std::get<0>(iter.value()); unsigned dim = iter.index(); - mlir::Value lb = lbounds.empty() ? one : lbounds[dim]; + mlir::Value lb = one; + if (!lbounds.empty()) { + lb = lbounds[dim]; + auto extentIsEmpty = rewriter.create( + loc, mlir::LLVM::ICmpPredicate::eq, extent, zero); + lb = rewriter.create(loc, extentIsEmpty, one, lb); + }; dest = insertLowerBound(rewriter, loc, dest, dim, lb); - dest = insertExtent(rewriter, loc, dest, dim, std::get<0>(iter.value())); + dest = insertExtent(rewriter, loc, dest, dim, extent); dest = insertStride(rewriter, loc, dest, dim, std::get<1>(iter.value())); } dest = insertBaseAddress(rewriter, loc, dest, base); diff --git a/flang/test/Fir/convert-to-llvm.fir b/flang/test/Fir/convert-to-llvm.fir index 6bf9a026cda69..85caecdd98f4b 100644 --- a/flang/test/Fir/convert-to-llvm.fir +++ b/flang/test/Fir/convert-to-llvm.fir @@ -1738,12 +1738,14 @@ func @xembox0(%arg0: !fir.ref>) { // CHECK: %[[ADJUSTED_OFFSET:.*]] = llvm.sub %[[C0]], %[[C0]] : i64 // CHECK: %[[DIM_OFFSET:.*]] = llvm.mul %[[ADJUSTED_OFFSET]], %[[ONE]] : i64 // CHECK: %[[PTR_OFFSET:.*]] = llvm.add %[[DIM_OFFSET]], %[[ZERO]] : i64 -// CHECK: %[[BOX7:.*]] = llvm.insertvalue %[[ZERO]], %[[BOX6]][7 : i32, 0 : i32, 0 : i32] : !llvm.struct<(ptr, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, array<1 x array<3 x i64>>)> // CHECK: %[[EXTENT0:.*]] = llvm.sub %[[C0]], %[[C0]] : i64 // CHECK: %[[EXTENT1:.*]] = llvm.add %[[EXTENT0]], %[[C0]] : i64 // CHECK: %[[EXTENT2:.*]] = llvm.sdiv %[[EXTENT1]], %[[C0]] : i64 // CHECK: %[[EXTENT_CMP:.*]] = llvm.icmp "sgt" %[[EXTENT2]], %[[ZERO]] : i64 // CHECK: %[[EXTENT:.*]] = llvm.select %[[EXTENT_CMP]], %[[EXTENT2]], %[[ZERO]] : i1, i64 +// CHECK: %[[EXTENT_CMP_2:.*]] = llvm.icmp "eq" %[[EXTENT]], %[[ZERO]] : i64 +// CHECK: %[[LOWER:.*]] = llvm.select %[[EXTENT_CMP_2]], %[[ONE]], %[[C0]] : i1, i64 +// CHECK: %[[BOX7:.*]] = llvm.insertvalue %[[LOWER]], %[[BOX6]][7 : i32, 0 : i32, 0 : i32] : !llvm.struct<(ptr, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, array<1 x array<3 x i64>>)> // CHECK: %[[BOX8:.*]] = llvm.insertvalue %[[EXTENT]], %[[BOX7]][7 : i32, 0 : i32, 1 : i32] : !llvm.struct<(ptr, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, array<1 x array<3 x i64>>)> // CHECK: %[[STRIDE:.*]] = llvm.mul %[[ELEM_LEN_I64]], %[[C0]] : i64 // CHECK: %[[BOX9:.*]] = llvm.insertvalue %[[STRIDE]], %[[BOX8]][7 : i32, 0 : i32, 2 : i32] : !llvm.struct<(ptr, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, array<1 x array<3 x i64>>)> @@ -1837,12 +1839,14 @@ func private @_QPxb(!fir.box>) // CHECK: %[[ADJUSTED_OFFSET:.*]] = llvm.sub %[[C2]], %[[SH1]] : i64 // CHECK: %[[DIM_OFFSET:.*]] = llvm.mul %[[ADJUSTED_OFFSET]], %[[ONE]] : i64 // CHECK: %[[PTR_OFFSET:.*]] = llvm.add %[[DIM_OFFSET]], %[[ZERO]] : i64 -// CHECK: %[[BOX7:.*]] = llvm.insertvalue %[[ZERO]], %[[BOX6]][7 : i32, 0 : i32, 0 : i32] : !llvm.struct<(ptr, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, array<2 x array<3 x i64>>)> // CHECK: %[[EXTENT0:.*]] = llvm.sub %[[ARG0]], %[[C2]] : i64 // CHECK: %[[EXTENT1:.*]] = llvm.add %[[EXTENT0]], %[[C1]] : i64 // CHECK: %[[EXTENT2:.*]] = llvm.sdiv %[[EXTENT1]], %[[C1]] : i64 // CHECK: %[[EXTENT_CMP:.*]] = llvm.icmp "sgt" %[[EXTENT2]], %[[ZERO]] : i64 // CHECK: %[[EXTENT:.*]] = llvm.select %[[EXTENT_CMP]], %[[EXTENT2]], %[[ZERO]] : i1, i64 +// CHECK: %[[EXTENT_CMP_2:.*]] = llvm.icmp "eq" %[[EXTENT]], %[[ZERO]] : i64 +// CHECK: %[[SH1B:.*]] = llvm.select %[[EXTENT_CMP_2]], %[[ONE]], %[[SH1]] : i1, i64 +// CHECK: %[[BOX7:.*]] = llvm.insertvalue %[[SH1B]], %[[BOX6]][7 : i32, 0 : i32, 0 : i32] : !llvm.struct<(ptr, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, array<2 x array<3 x i64>>)> // CHECK: %[[BOX8:.*]] = llvm.insertvalue %[[EXTENT]], %[[BOX7]][7 : i32, 0 : i32, 1 : i32] : !llvm.struct<(ptr, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, array<2 x array<3 x i64>>)> // CHECK: %[[STRIDE:.*]] = llvm.mul %[[ELEM_LEN_I64]], %[[C1]] : i64 // CHECK: %[[BOX9:.*]] = llvm.insertvalue %[[STRIDE]], %[[BOX8]][7 : i32, 0 : i32, 2 : i32] : !llvm.struct<(ptr, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, array<2 x array<3 x i64>>)> @@ -1851,12 +1855,14 @@ func private @_QPxb(!fir.box>) // CHECK: %[[ADJUSTED_OFFSET:.*]] = llvm.sub %[[C4]], %[[SH2]] : i64 // CHECK: %[[DIM_OFFSET:.*]] = llvm.mul %[[ADJUSTED_OFFSET]], %[[PREV_PTROFF]] : i64 // CHECK: %[[PTR_OFFSET0:.*]] = llvm.add %[[DIM_OFFSET]], %[[PTR_OFFSET]] : i64 -// CHECK: %[[BOX10:.*]] = llvm.insertvalue %[[ZERO]], %[[BOX9]][7 : i32, 1 : i32, 0 : i32] : !llvm.struct<(ptr, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, array<2 x array<3 x i64>>)> // CHECK: %[[EXT_SUB:.*]] = llvm.sub %[[N]], %[[C4]] : i64 // CHECK: %[[EXT_ADD:.*]] = llvm.add %[[EXT_SUB]], %[[C1]] : i64 // CHECK: %[[EXT_SDIV:.*]] = llvm.sdiv %[[EXT_ADD]], %[[C1]] : i64 // CHECK: %[[EXT_ICMP:.*]] = llvm.icmp "sgt" %[[EXT_SDIV]], %[[ZERO]] : i64 // CHECK: %[[EXT_SELECT:.*]] = llvm.select %[[EXT_ICMP]], %[[EXT_SDIV]], %[[ZERO]] : i1, i64 +// CHECK: %[[EXT_ICMP_2:.*]] = llvm.icmp "eq" %[[EXT_SELECT]], %[[ZERO]] : i64 +// CHECK: %[[SH2B:.*]] = llvm.select %[[EXT_ICMP_2]], %[[ONE]], %[[SH2]] : i1, i64 +// CHECK: %[[BOX10:.*]] = llvm.insertvalue %[[SH2B]], %[[BOX9]][7 : i32, 1 : i32, 0 : i32] : !llvm.struct<(ptr, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, array<2 x array<3 x i64>>)> // CHECK: %[[BOX11:.*]] = llvm.insertvalue %[[EXT_SELECT]], %[[BOX10]][7 : i32, 1 : i32, 1 : i32] : !llvm.struct<(ptr, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, array<2 x array<3 x i64>>)> // CHECK: %[[STRIDE_MUL:.*]] = llvm.mul %[[PREV_DIM]], %[[C1]] : i64 // CHECK: %[[BOX12:.*]] = llvm.insertvalue %[[STRIDE_MUL]], %[[BOX11]][7 : i32, 1 : i32, 2 : i32] : !llvm.struct<(ptr, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, array<2 x array<3 x i64>>)> @@ -1918,12 +1924,12 @@ func private @_QPtest_dt_callee(%arg0: !fir.box>) // CHECK: %[[GEP_DTYPE_SIZE:.*]] = llvm.getelementptr %[[ELE_TYPE]][%[[C1_0]]] : (!llvm.ptr>, i64) -> !llvm.ptr> // CHECK: %[[PTRTOINT_DTYPE_SIZE:.*]] = llvm.ptrtoint %[[GEP_DTYPE_SIZE]] : !llvm.ptr> to i64 // CHECK: %[[ADJUSTED_OFFSET:.*]] = llvm.sub %3, %30 : i64 -// CHECK: %[[BOX7:.*]] = llvm.insertvalue %[[ZERO]], %[[BOX6]][7 : i32, 0 : i32, 0 : i32] : !llvm.struct<(ptr, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, array<1 x array<3 x i64>>)> // CHECK: %[[EXT_SUB:.*]] = llvm.sub %[[C10]], %[[C1]] : i64 // CHECK: %[[EXT_ADD:.*]] = llvm.add %[[EXT_SUB]], %[[C2]] : i64 // CHECK: %[[EXT_SDIV:.*]] = llvm.sdiv %[[EXT_ADD]], %[[C2]] : i64 // CHECK: %[[EXT_ICMP:.*]] = llvm.icmp "sgt" %[[EXT_SDIV]], %[[ZERO]] : i64 // CHECK: %[[EXT_SELECT:.*]] = llvm.select %[[EXT_ICMP]], %[[EXT_SDIV]], %[[ZERO]] : i1, i64 +// CHECK: %[[BOX7:.*]] = llvm.insertvalue %[[ONE]], %[[BOX6]][7 : i32, 0 : i32, 0 : i32] : !llvm.struct<(ptr, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, array<1 x array<3 x i64>>)> // CHECK: %[[BOX8:.*]] = llvm.insertvalue %[[EXT_SELECT]], %[[BOX7]][7 : i32, 0 : i32, 1 : i32] : !llvm.struct<(ptr, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, array<1 x array<3 x i64>>)> // CHECK: %[[STRIDE_MUL:.*]] = llvm.mul %[[PTRTOINT_DTYPE_SIZE]], %[[C2]] : i64 // CHECK: %[[BOX9:.*]] = llvm.insertvalue %[[STRIDE_MUL]], %[[BOX8]][7 : i32, 0 : i32, 2 : i32] : !llvm.struct<(ptr, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, array<1 x array<3 x i64>>)>