https://github.com/ergawy updated https://github.com/llvm/llvm-project/pull/227625
>From 926164538762be44d04ccd23b89c2dd523378eb8 Mon Sep 17 00:00:00 2001 From: ergawy <[email protected]> Date: Wed, 30 Sep 2026 01:54:28 -0700 Subject: [PATCH] [flang][codegen] Report a shape or slice cg-rewrite cannot read cg-rewrite folds a fir.shape, fir.shape_shift, fir.shift or fir.slice into the code-gen form by reading it through its defining op. A value that has none cannot be folded. For a slice this went unreported: the rewrite dropped it and produced a descriptor for the whole array rather than the section it names. For a shape it reached a cast on a null defining op. Report it instead, and say which operand. Rebuilding the value covers a block argument, but not every case: a slice chosen by an arith.select has no single value to take apart. --- flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp | 58 ++++++++++++++----- .../test/Fir/cg-rewrite-unfoldable-slice.fir | 25 ++++++++ 2 files changed, 70 insertions(+), 13 deletions(-) create mode 100644 flang/test/Fir/cg-rewrite-unfoldable-slice.fir diff --git a/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp b/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp index 36430c440f234..66954f17acf56 100644 --- a/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp +++ b/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp @@ -57,6 +57,25 @@ static void populateShift(llvm::SmallVectorImpl<mlir::Value> &vec, vec.append(shift.getOrigins().begin(), shift.getOrigins().end()); } +/// Report a shape or slice the rewrites cannot read. +/// +/// They are folded into the code-gen form through their defining op, so a +/// value that has none -- a block argument, or the result of a select -- +/// cannot be folded. Saying so is better than what the rewrites would +/// otherwise do: describe the whole array instead of the section it names. +static llvm::LogicalResult checkFoldableShapeAndSlice(mlir::Operation *op, + mlir::Value shape, + mlir::Value slice) { + if (shape && + !mlir::isa_and_nonnull<fir::ShapeOp, fir::ShapeShiftOp, fir::ShiftOp>( + shape.getDefiningOp())) + return op->emitOpError("shape operand is not defined by a fir.shape, " + "fir.shape_shift or fir.shift"); + if (slice && !mlir::isa_and_nonnull<fir::SliceOp>(slice.getDefiningOp())) + return op->emitOpError("slice operand is not defined by a fir.slice"); + return llvm::success(); +} + // Helper to emit embox/rebox for OPTIONAL input inside a block // guarded by a runtime presence check and to return an absent // box when the input is not present. @@ -136,6 +155,10 @@ class EmboxConversion : public mlir::OpRewritePattern<fir::EmboxOp> { llvm::FailureOr<RewriteKind> rewriteKind = getRewriteKind(embox); if (llvm::failed(rewriteKind)) return llvm::failure(); + if (*rewriteKind == RewriteKind::Dynamic && + llvm::failed(checkFoldableShapeAndSlice(embox, embox.getShape(), + embox.getSlice()))) + return llvm::failure(); if (embox.getOptional()) { mlir::Value newBox = emitOptionalBoxGuard(rewriter, embox, [&] { return matchAndRewriteImpl(embox, rewriter, *rewriteKind)->getResult(0); @@ -194,27 +217,30 @@ class EmboxConversion : public mlir::OpRewritePattern<fir::EmboxOp> { auto loc = embox.getLoc(); llvm::SmallVector<mlir::Value> shapeOpers; llvm::SmallVector<mlir::Value> shiftOpers; - if (auto shapeOp = mlir::dyn_cast<fir::ShapeOp>(shapeVal.getDefiningOp())) { + // matchAndRewrite has already reported a shape this cannot read. + mlir::Operation *shapeDef = shapeVal.getDefiningOp(); + if (auto shapeOp = mlir::dyn_cast_or_null<fir::ShapeOp>(shapeDef)) { populateShape(shapeOpers, shapeOp); } else { - auto shiftOp = - mlir::dyn_cast<fir::ShapeShiftOp>(shapeVal.getDefiningOp()); + auto shiftOp = mlir::dyn_cast_or_null<fir::ShapeShiftOp>(shapeDef); assert(shiftOp && "shape is neither fir.shape nor fir.shape_shift"); populateShapeAndShift(shapeOpers, shiftOpers, shiftOp); } llvm::SmallVector<mlir::Value> sliceOpers; llvm::SmallVector<mlir::Value> subcompOpers; llvm::SmallVector<mlir::Value> substrOpers; - if (auto s = embox.getSlice()) - if (auto sliceOp = - mlir::dyn_cast_or_null<fir::SliceOp>(s.getDefiningOp())) { - sliceOpers.assign(sliceOp.getTriples().begin(), - sliceOp.getTriples().end()); - subcompOpers.assign(sliceOp.getFields().begin(), - sliceOp.getFields().end()); - substrOpers.assign(sliceOp.getSubstr().begin(), - sliceOp.getSubstr().end()); - } + if (auto s = embox.getSlice()) { + auto sliceOp = mlir::dyn_cast_or_null<fir::SliceOp>(s.getDefiningOp()); + // matchAndRewrite has already reported a slice this cannot read. + // Dropping it would describe the whole array instead of the section. + assert(sliceOp && "slice is not defined by a fir.slice"); + sliceOpers.assign(sliceOp.getTriples().begin(), + sliceOp.getTriples().end()); + subcompOpers.assign(sliceOp.getFields().begin(), + sliceOp.getFields().end()); + substrOpers.assign(sliceOp.getSubstr().begin(), + sliceOp.getSubstr().end()); + } auto xbox = fir::cg::XEmboxOp::create( rewriter, loc, embox.getType(), embox.getMemref(), shapeOpers, shiftOpers, sliceOpers, subcompOpers, substrOpers, @@ -244,6 +270,9 @@ class ReboxConversion : public mlir::OpRewritePattern<fir::ReboxOp> { llvm::LogicalResult matchAndRewrite(fir::ReboxOp rebox, mlir::PatternRewriter &rewriter) const override { + if (llvm::failed(checkFoldableShapeAndSlice(rebox, rebox.getShape(), + rebox.getSlice()))) + return llvm::failure(); if (rebox.getOptional()) { mlir::Value newBox = emitOptionalBoxGuard(rewriter, rebox, [&] { return matchAndRewriteImpl(rebox, rewriter)->getResult(0); @@ -316,6 +345,9 @@ class ArrayCoorConversion : public mlir::OpRewritePattern<fir::ArrayCoorOp> { llvm::LogicalResult matchAndRewrite(fir::ArrayCoorOp arrCoor, mlir::PatternRewriter &rewriter) const override { + if (llvm::failed(checkFoldableShapeAndSlice(arrCoor, arrCoor.getShape(), + arrCoor.getSlice()))) + return llvm::failure(); auto loc = arrCoor.getLoc(); llvm::SmallVector<mlir::Value> shapeOpers; llvm::SmallVector<mlir::Value> shiftOpers; diff --git a/flang/test/Fir/cg-rewrite-unfoldable-slice.fir b/flang/test/Fir/cg-rewrite-unfoldable-slice.fir new file mode 100644 index 0000000000000..bcf71244ee08f --- /dev/null +++ b/flang/test/Fir/cg-rewrite-unfoldable-slice.fir @@ -0,0 +1,25 @@ +// A slice that reaches fir.embox through an arith.select has no fir.slice to +// read it from, and unlike a block argument there is nothing to take apart. +// Report it: folding it away would describe the whole array rather than the +// section the slice names. + +// RUN: not fir-opt %s -cg-rewrite 2>&1 | FileCheck %s + +func.func private @use(!fir.box<!fir.array<?xf64>>) + +// CHECK: 'fir.embox' op slice operand is not defined by a fir.slice +func.func @selected_slice(%cond: i1) { + %c1 = arith.constant 1 : index + %c2 = arith.constant 2 : index + %c3 = arith.constant 3 : index + %c4 = arith.constant 4 : index + %shift = fir.shape_shift %c1, %c4 : (index, index) -> !fir.shapeshift<1> + %heap = fir.allocmem !fir.array<?xf64>, %c4 + %slice1 = fir.slice %c1, %c2, %c1 : (index, index, index) -> !fir.slice<1> + %slice2 = fir.slice %c1, %c3, %c1 : (index, index, index) -> !fir.slice<1> + %slice = arith.select %cond, %slice1, %slice2 : !fir.slice<1> + %emb = fir.embox %heap(%shift) [%slice] : (!fir.heap<!fir.array<?xf64>>, !fir.shapeshift<1>, !fir.slice<1>) -> !fir.box<!fir.array<?xf64>> + func.call @use(%emb) : (!fir.box<!fir.array<?xf64>>) -> () + fir.freemem %heap : !fir.heap<!fir.array<?xf64>> + return +} _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
