Skip to content

Commit

Permalink
[flang][codegen] ensure descriptor lower bounds are LBOUND compliant
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@jeanPerier
jeanPerier committed Mar 25, 2022
1 parent 7f1adba commit d3bc3a0
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 14 deletions.
4 changes: 4 additions & 0 deletions flang/include/flang/Optimizer/Dialect/FIROps.h
View file
Expand Up @@ -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
Expand Down
46 changes: 36 additions & 10 deletions flang/lib/Optimizer/CodeGen/CodeGen.cpp
View file
Expand Up @@ -1529,20 +1529,30 @@ struct XEmboxOpConversion : public EmboxCommonConversion<fir::cg::XEmboxOp> {
}
}
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<fir::PointerType>() || eleTy.isa<fir::HeapType>()) {
const bool isaPointerOrAllocatable =
eleTy.isa<fir::PointerType>() || eleTy.isa<fir::HeapType>();
// 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<mlir::LLVM::ICmpOp>(
loc, mlir::LLVM::ICmpPredicate::eq, extent, zero);
lb = rewriter.create<mlir::LLVM::SelectOp>(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)
Expand Down Expand Up @@ -1597,6 +1607,13 @@ struct XEmboxOpConversion : public EmboxCommonConversion<fir::cg::XEmboxOp> {
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.
Expand Down Expand Up @@ -1662,12 +1679,21 @@ struct XReboxOpConversion : public EmboxCommonConversion<fir::cg::XReboxOp> {
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<mlir::LLVM::ICmpOp>(
loc, mlir::LLVM::ICmpPredicate::eq, extent, zero);
lb = rewriter.create<mlir::LLVM::SelectOp>(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);
Expand Down
14 changes: 10 additions & 4 deletions flang/test/Fir/convert-to-llvm.fir
View file
Expand Up @@ -1738,12 +1738,14 @@ func @xembox0(%arg0: !fir.ref<!fir.array<?xi32>>) {
// 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<i32>, 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<i32>, 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<i32>, 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<i32>, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, array<1 x array<3 x i64>>)>
Expand Down Expand Up @@ -1837,12 +1839,14 @@ func private @_QPxb(!fir.box<!fir.array<?x?xf64>>)
// 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<f64>, 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<f64>, 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<f64>, 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<f64>, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, array<2 x array<3 x i64>>)>
Expand All @@ -1851,12 +1855,14 @@ func private @_QPxb(!fir.box<!fir.array<?x?xf64>>)
// 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<f64>, 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<f64>, 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<f64>, 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<f64>, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, array<2 x array<3 x i64>>)>
Expand Down Expand Up @@ -1918,12 +1924,12 @@ func private @_QPtest_dt_callee(%arg0: !fir.box<!fir.array<?xi32>>)
// CHECK: %[[GEP_DTYPE_SIZE:.*]] = llvm.getelementptr %[[ELE_TYPE]][%[[C1_0]]] : (!llvm.ptr<struct<"_QFtest_dt_sliceTt", (i32, i32)>>, i64) -> !llvm.ptr<struct<"_QFtest_dt_sliceTt", (i32, i32)>>
// CHECK: %[[PTRTOINT_DTYPE_SIZE:.*]] = llvm.ptrtoint %[[GEP_DTYPE_SIZE]] : !llvm.ptr<struct<"_QFtest_dt_sliceTt", (i32, i32)>> 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<i32>, 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<i32>, 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<i32>, 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<i32>, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, array<1 x array<3 x i64>>)>
Expand Down

0 comments on commit d3bc3a0

Please sign in to comment.