[flang-commits] [flang] 649439e - [flang] Fix lowering issue with character temp
Valentin Clement via flang-commits
flang-commits at lists.llvm.org
Wed Jun 29 11:07:02 PDT 2022
Author: Valentin Clement
Date: 2022-06-29T20:06:54+02:00
New Revision: 649439e7aeeb3b30f54297b3c6f7d6549fb2f85a
URL: https://github.com/llvm/llvm-project/commit/649439e7aeeb3b30f54297b3c6f7d6549fb2f85a
DIFF: https://github.com/llvm/llvm-project/commit/649439e7aeeb3b30f54297b3c6f7d6549fb2f85a.diff
LOG: [flang] Fix lowering issue with character temp
- Add verifiers that determine if an Op requires type parameters or
not and checks that the correct number of parameters is specified.
This patch is part of the upstreaming effort from fir-dev branch.
Reviewed By: PeteSteinfeld
Differential Revision: https://reviews.llvm.org/D128828
Co-authored-by: Eric Schweitz <eschweitz at nvidia.com>
Added:
Modified:
flang/lib/Lower/ConvertExpr.cpp
flang/lib/Optimizer/CodeGen/CodeGen.cpp
flang/lib/Optimizer/Dialect/FIROps.cpp
flang/lib/Optimizer/Transforms/ArrayValueCopy.cpp
flang/test/Lower/forall/character-1.f90
Removed:
################################################################################
diff --git a/flang/lib/Lower/ConvertExpr.cpp b/flang/lib/Lower/ConvertExpr.cpp
index df216adee4c53..5771a89015ac3 100644
--- a/flang/lib/Lower/ConvertExpr.cpp
+++ b/flang/lib/Lower/ConvertExpr.cpp
@@ -2593,6 +2593,12 @@ class ScalarExprLowering {
cast = builder.create<fir::EmboxProcOp>(loc, boxProcTy, fst);
}
} else {
+ if (fir::isa_derived(snd)) {
+ // FIXME: This seems like a serious bug elsewhere in lowering. Paper
+ // over the problem for now.
+ TODO(loc, "derived type argument passed by value");
+ }
+ assert(!fir::isa_derived(snd));
cast = builder.convertWithSemantics(loc, snd, fst,
callingImplicitInterface);
}
@@ -4620,7 +4626,7 @@ class ArrayExprLowering {
auto seqTy = type.dyn_cast<fir::SequenceType>();
assert(seqTy && "must be an array");
mlir::Location loc = getLoc();
- // TODO: Need to thread the length parameters here. For character, they may
+ // TODO: Need to thread the LEN parameters here. For character, they may
//
diff er from the operands length (e.g concatenation). So the array loads
// type parameters are not enough.
if (auto charTy = seqTy.getEleTy().dyn_cast<fir::CharacterType>())
@@ -6137,7 +6143,7 @@ class ArrayExprLowering {
mlir::Value multiplier = builder.createIntegerConstant(loc, idxTy, 1);
if (fir::hasDynamicSize(eleTy)) {
if (auto charTy = eleTy.dyn_cast<fir::CharacterType>()) {
- // Array of char with dynamic length parameter. Downcast to an array
+ // Array of char with dynamic LEN parameter. Downcast to an array
// of singleton char, and scale by the len type parameter from
// `exv`.
exv.match(
diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
index 2a5f126b684b2..abd1c123cb5c4 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
@@ -2185,16 +2185,11 @@ struct XArrayCoorOpConversion
assert(eleTy && "result must be a reference-like type");
if (fir::characterWithDynamicLen(eleTy)) {
assert(coor.lenParams().size() == 1);
- auto bitsInChar = lowerTy().getKindMap().getCharacterBitsize(
- eleTy.cast<fir::CharacterType>().getFKind());
- auto scaling = genConstantIndex(loc, idxTy, rewriter, bitsInChar / 8);
- auto scaledBySize =
- rewriter.create<mlir::LLVM::MulOp>(loc, idxTy, offset, scaling);
- auto length =
- integerCast(loc, rewriter, idxTy,
- adaptor.getOperands()[coor.lenParamsOffset()]);
- offset = rewriter.create<mlir::LLVM::MulOp>(loc, idxTy, scaledBySize,
- length);
+ auto length = integerCast(loc, rewriter, idxTy,
+ operands[coor.lenParamsOffset()]);
+ offset =
+ rewriter.create<mlir::LLVM::MulOp>(loc, idxTy, offset, length);
+
} else {
TODO(loc, "compute size of derived type with type parameters");
}
diff --git a/flang/lib/Optimizer/Dialect/FIROps.cpp b/flang/lib/Optimizer/Dialect/FIROps.cpp
index 8d0868dc5f035..38cb0475c7939 100644
--- a/flang/lib/Optimizer/Dialect/FIROps.cpp
+++ b/flang/lib/Optimizer/Dialect/FIROps.cpp
@@ -344,6 +344,24 @@ mlir::LogicalResult fir::AllocMemOp::verify() {
// ArrayCoorOp
//===----------------------------------------------------------------------===//
+// CHARACTERs and derived types with LEN PARAMETERs are dependent types that
+// require runtime values to fully define the type of an object.
+static bool validTypeParams(mlir::Type dynTy, mlir::ValueRange typeParams) {
+ dynTy = fir::unwrapAllRefAndSeqType(dynTy);
+ // A box value will contain type parameter values itself.
+ if (dynTy.isa<fir::BoxType>())
+ return typeParams.size() == 0;
+ // Derived type must have all type parameters satisfied.
+ if (auto recTy = dynTy.dyn_cast<fir::RecordType>())
+ return typeParams.size() == recTy.getNumLenParams();
+ // Characters with non-constant LEN must have a type parameter value.
+ if (auto charTy = dynTy.dyn_cast<fir::CharacterType>())
+ if (charTy.hasDynamicLen())
+ return typeParams.size() == 1;
+ // Otherwise, any type parameters are invalid.
+ return typeParams.size() == 0;
+}
+
mlir::LogicalResult fir::ArrayCoorOp::verify() {
auto eleTy = fir::dyn_cast_ptrOrBoxEleTy(getMemref().getType());
auto arrTy = eleTy.dyn_cast<fir::SequenceType>();
@@ -378,6 +396,8 @@ mlir::LogicalResult fir::ArrayCoorOp::verify() {
if (sliceTy.getRank() != arrDim)
return emitOpError("rank of dimension in slice mismatched");
}
+ if (!validTypeParams(getMemref().getType(), getTypeparams()))
+ return emitOpError("invalid type parameters");
return mlir::success();
}
@@ -444,6 +464,9 @@ mlir::LogicalResult fir::ArrayLoadOp::verify() {
return emitOpError("rank of dimension in slice mismatched");
}
+ if (!validTypeParams(getMemref().getType(), getTypeparams()))
+ return emitOpError("invalid type parameters");
+
return mlir::success();
}
@@ -485,6 +508,8 @@ mlir::LogicalResult fir::ArrayMergeStoreOp::verify() {
return emitOpError("type of origin does not match memref element type");
if (getSequence().getType() != eleTy)
return emitOpError("type of sequence does not match memref element type");
+ if (!validTypeParams(getMemref().getType(), getTypeparams()))
+ return emitOpError("invalid type parameters");
return mlir::success();
}
@@ -512,6 +537,8 @@ mlir::LogicalResult fir::ArrayFetchOp::verify() {
return emitOpError("return type and/or indices do not type check");
if (!mlir::isa<fir::ArrayLoadOp>(getSequence().getDefiningOp()))
return emitOpError("argument #0 must be result of fir.array_load");
+ if (!validTypeParams(arrTy, getTypeparams()))
+ return emitOpError("invalid type parameters");
return mlir::success();
}
@@ -530,6 +557,8 @@ mlir::LogicalResult fir::ArrayAccessOp::verify() {
mlir::Type ty = validArraySubobject(*this);
if (!ty || fir::ReferenceType::get(ty) != getType())
return emitOpError("return type and/or indices do not type check");
+ if (!validTypeParams(arrTy, getTypeparams()))
+ return emitOpError("invalid type parameters");
return mlir::success();
}
@@ -550,6 +579,8 @@ mlir::LogicalResult fir::ArrayUpdateOp::verify() {
auto ty = validArraySubobject(*this);
if (!ty || ty != ::adjustedElementType(getMerge().getType()))
return emitOpError("merged value and/or indices do not type check");
+ if (!validTypeParams(arrTy, getTypeparams()))
+ return emitOpError("invalid type parameters");
return mlir::success();
}
@@ -689,6 +720,24 @@ void fir::CallOp::build(mlir::OpBuilder &builder, mlir::OperationState &result,
result.addTypes(results);
}
+//===----------------------------------------------------------------------===//
+// CharConvertOp
+//===----------------------------------------------------------------------===//
+
+mlir::LogicalResult fir::CharConvertOp::verify() {
+ auto unwrap = [&](mlir::Type t) {
+ t = fir::unwrapSequenceType(fir::dyn_cast_ptrEleTy(t));
+ return t.dyn_cast<fir::CharacterType>();
+ };
+ auto inTy = unwrap(getFrom().getType());
+ auto outTy = unwrap(getTo().getType());
+ if (!(inTy && outTy))
+ return emitOpError("not a reference to a character");
+ if (inTy.getFKind() == outTy.getFKind())
+ return emitOpError("buffers must have
diff erent KIND values");
+ return mlir::success();
+}
+
//===----------------------------------------------------------------------===//
// CmpOp
//===----------------------------------------------------------------------===//
@@ -742,24 +791,6 @@ static mlir::ParseResult parseCmpOp(mlir::OpAsmParser &parser,
return mlir::success();
}
-//===----------------------------------------------------------------------===//
-// CharConvertOp
-//===----------------------------------------------------------------------===//
-
-mlir::LogicalResult fir::CharConvertOp::verify() {
- auto unwrap = [&](mlir::Type t) {
- t = fir::unwrapSequenceType(fir::dyn_cast_ptrEleTy(t));
- return t.dyn_cast<fir::CharacterType>();
- };
- auto inTy = unwrap(getFrom().getType());
- auto outTy = unwrap(getTo().getType());
- if (!(inTy && outTy))
- return emitOpError("not a reference to a character");
- if (inTy.getFKind() == outTy.getFKind())
- return emitOpError("buffers must have
diff erent KIND values");
- return mlir::success();
-}
-
//===----------------------------------------------------------------------===//
// CmpcOp
//===----------------------------------------------------------------------===//
diff --git a/flang/lib/Optimizer/Transforms/ArrayValueCopy.cpp b/flang/lib/Optimizer/Transforms/ArrayValueCopy.cpp
index 019401026ba16..37af33f0a5ead 100644
--- a/flang/lib/Optimizer/Transforms/ArrayValueCopy.cpp
+++ b/flang/lib/Optimizer/Transforms/ArrayValueCopy.cpp
@@ -969,7 +969,7 @@ void genArrayCopy(mlir::Location loc, mlir::PatternRewriter &rewriter,
loc, getEleTy(dst.getType()), dst, shapeOp,
!CopyIn && copyUsingSlice ? sliceOp : mlir::Value{},
factory::originateIndices(loc, rewriter, dst.getType(), shapeOp, indices),
- getTypeParamsIfRawData(loc, builder, arrLoad, src.getType()));
+ getTypeParamsIfRawData(loc, builder, arrLoad, dst.getType()));
auto eleTy = unwrapSequenceType(unwrapPassByRefType(dst.getType()));
// Copy from (to) object to (from) temp copy of same object.
if (auto charTy = eleTy.dyn_cast<CharacterType>()) {
diff --git a/flang/test/Lower/forall/character-1.f90 b/flang/test/Lower/forall/character-1.f90
index 92f215b23a889..01f1257bb9b86 100644
--- a/flang/test/Lower/forall/character-1.f90
+++ b/flang/test/Lower/forall/character-1.f90
@@ -18,7 +18,7 @@ end subroutine sub
end program test
! CHECK-LABEL: define void @_QFPsub(
-! CHECK-SAME: ptr %[[arg:.*]])
+! CHECK-SAME: ptr %[[arg:.*]])
! CHECK: %[[extent:.*]] = getelementptr { {{.*}}, [1 x [3 x i64]] }, ptr %[[arg]], i32 0, i32 7, i64 0, i32 1
! CHECK: %[[extval:.*]] = load i64, ptr %[[extent]]
! CHECK: %[[elesize:.*]] = getelementptr { {{.*}}, [1 x [3 x i64]] }, ptr %[[arg]], i32 0, i32 1
More information about the flang-commits
mailing list