[flang][codegen] Update FIR codegen to use mlir.llvm opaque pointers #69692
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jeanPerier
requested review from
tblah,
kiranchandramohan,
zero9178,
TIFitis and
vzakhari
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-codegen @llvm/pr-subscribers-flang-openmp Author: None (jeanPerier) Changes!llvm.ptr<T> typed pointers are depreciated in MLIR LLVM dialects. Flang codegen still generated them and relied on mlir.llvm codegen to LLVM to turn them into opaque pointers. This patch update FIR codegen to directly emit and work with LLVM opaque pointers. Addresses #69303 @kiranchandramohan and @TIFitis, this patch cannot be merged currently because the MLIR OpenMP dialect OpenMPToLLVMIRTranslation codegen pass relies on getting typed pointers (* see 1 below) and needs to be updated first.
Patch is 328.92 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/69692.diff 19 Files Affected:
diff --git a/flang/include/flang/Optimizer/CodeGen/TypeConverter.h b/flang/include/flang/Optimizer/CodeGen/TypeConverter.h
index 29d0a902f556269..9ce756bdfd96611 100644
--- a/flang/include/flang/Optimizer/CodeGen/TypeConverter.h
+++ b/flang/include/flang/Optimizer/CodeGen/TypeConverter.h
@@ -74,7 +74,7 @@ class LLVMTypeConverter : public mlir::LLVMTypeConverter {
/// Convert fir.box type to the corresponding llvm struct type instead of a
/// pointer to this struct type.
- mlir::Type convertBoxTypeAsStruct(BaseBoxType box) const;
+ mlir::Type convertBoxTypeAsStruct(BaseBoxType box, int = unknownRank()) const;
// fir.boxproc<any> --> llvm<"{ any*, i8* }">
mlir::Type convertBoxProcType(BoxProcType boxproc) const;
@@ -97,29 +97,7 @@ class LLVMTypeConverter : public mlir::LLVMTypeConverter {
}
template <typename A> mlir::Type convertPointerLike(A &ty) const {
- mlir::Type eleTy = ty.getEleTy();
- // A sequence type is a special case. A sequence of runtime size on its
- // interior dimensions lowers to a memory reference. In that case, we
- // degenerate the array and do not want a the type to become `T**` but
- // merely `T*`.
- if (auto seqTy = eleTy.dyn_cast<fir::SequenceType>()) {
- if (seqTy.hasDynamicExtents() ||
- characterWithDynamicLen(seqTy.getEleTy())) {
- if (seqTy.getConstantRows() > 0)
- return convertType(seqTy);
- eleTy = seqTy.getEleTy();
- }
- }
- // fir.ref<fir.box> is a special case because fir.box type is already
- // a pointer to a Fortran descriptor at the LLVM IR level. This implies
- // that a fir.ref<fir.box>, that is the address of fir.box is actually
- // the same as a fir.box at the LLVM level.
- // The distinction is kept in fir to denote when a descriptor is expected
- // to be mutable (fir.ref<fir.box>) and when it is not (fir.box).
- if (eleTy.isa<fir::BaseBoxType>())
- return convertType(eleTy);
-
- return mlir::LLVM::LLVMPointerType::get(convertType(eleTy));
+ return mlir::LLVM::LLVMPointerType::get(ty.getContext());
}
// convert a front-end kind value to either a std or LLVM IR dialect type
diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
index f17df3f5c6fc636..36b4e99d5ae2326 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
@@ -63,8 +63,12 @@ static constexpr unsigned defaultAlign = 8;
static constexpr unsigned kAttrPointer = CFI_attribute_pointer;
static constexpr unsigned kAttrAllocatable = CFI_attribute_allocatable;
-static inline mlir::Type getVoidPtrType(mlir::MLIRContext *context) {
- return mlir::LLVM::LLVMPointerType::get(mlir::IntegerType::get(context, 8));
+static inline mlir::Type getLlvmPtrType(mlir::MLIRContext *context) {
+ return mlir::LLVM::LLVMPointerType::get(context);
+}
+
+static inline mlir::Type getI8Type(mlir::MLIRContext *context) {
+ return mlir::IntegerType::get(context, 8);
}
static mlir::LLVM::ConstantOp
@@ -125,11 +129,13 @@ class FIROpConversion : public mlir::ConvertOpToLLVMPattern<FromOp> {
mlir::Type convertType(mlir::Type ty) const {
return lowerTy().convertType(ty);
}
- mlir::Type voidPtrTy() const { return getVoidPtrType(); }
- mlir::Type getVoidPtrType() const {
- return mlir::LLVM::LLVMPointerType::get(
- mlir::IntegerType::get(&lowerTy().getContext(), 8));
+ // Convert FIR type to LLVM without turning fir.box<T> into memory
+ // reference.
+ mlir::Type convertObjectType(mlir::Type firType) const {
+ if (auto boxTy = firType.dyn_cast<fir::BaseBoxType>())
+ return lowerTy().convertBoxTypeAsStruct(boxTy);
+ return lowerTy().convertType(firType);
}
mlir::LLVM::ConstantOp
@@ -170,17 +176,29 @@ class FIROpConversion : public mlir::ConvertOpToLLVMPattern<FromOp> {
return val;
}
+ struct TypePair {
+ mlir::Type fir;
+ mlir::Type llvm;
+ };
+
+ TypePair getBoxTypePair(mlir::Type firBoxTy) const {
+ mlir::Type llvmBoxTy = lowerTy().convertBoxTypeAsStruct(
+ mlir::cast<fir::BaseBoxType>(firBoxTy));
+ return TypePair{firBoxTy, llvmBoxTy};
+ }
+
/// Construct code sequence to extract the specific value from a `fir.box`.
- mlir::Value getValueFromBox(mlir::Location loc, mlir::Type boxTy,
+ mlir::Value getValueFromBox(mlir::Location loc, TypePair boxTy,
mlir::Value box, mlir::Type resultTy,
mlir::ConversionPatternRewriter &rewriter,
int boxValue) const {
if (box.getType().isa<mlir::LLVM::LLVMPointerType>()) {
- auto pty = mlir::LLVM::LLVMPointerType::get(resultTy);
+ auto pty = ::getLlvmPtrType(resultTy.getContext());
auto p = rewriter.create<mlir::LLVM::GEPOp>(
- loc, pty, box, llvm::ArrayRef<mlir::LLVM::GEPArg>{0, boxValue});
+ loc, pty, boxTy.llvm, box,
+ llvm::ArrayRef<mlir::LLVM::GEPArg>{0, boxValue});
auto loadOp = rewriter.create<mlir::LLVM::LoadOp>(loc, resultTy, p);
- attachTBAATag(loadOp, boxTy, nullptr, p);
+ attachTBAATag(loadOp, boxTy.fir, nullptr, p);
return loadOp;
}
return rewriter.create<mlir::LLVM::ExtractValueOp>(loc, box, boxValue);
@@ -190,7 +208,7 @@ class FIROpConversion : public mlir::ConvertOpToLLVMPattern<FromOp> {
/// from a box.
llvm::SmallVector<mlir::Value, 3>
getDimsFromBox(mlir::Location loc, llvm::ArrayRef<mlir::Type> retTys,
- mlir::Type boxTy, mlir::Value box, mlir::Value dim,
+ TypePair boxTy, mlir::Value box, mlir::Value dim,
mlir::ConversionPatternRewriter &rewriter) const {
mlir::Value l0 =
loadDimFieldFromBox(loc, boxTy, box, dim, 0, retTys[0], rewriter);
@@ -203,7 +221,7 @@ class FIROpConversion : public mlir::ConvertOpToLLVMPattern<FromOp> {
llvm::SmallVector<mlir::Value, 3>
getDimsFromBox(mlir::Location loc, llvm::ArrayRef<mlir::Type> retTys,
- mlir::Type boxTy, mlir::Value box, int dim,
+ TypePair boxTy, mlir::Value box, int dim,
mlir::ConversionPatternRewriter &rewriter) const {
mlir::Value l0 =
getDimFieldFromBox(loc, boxTy, box, dim, 0, retTys[0], rewriter);
@@ -215,30 +233,28 @@ class FIROpConversion : public mlir::ConvertOpToLLVMPattern<FromOp> {
}
mlir::Value
- loadDimFieldFromBox(mlir::Location loc, mlir::Type boxTy, mlir::Value box,
+ loadDimFieldFromBox(mlir::Location loc, TypePair boxTy, mlir::Value box,
mlir::Value dim, int off, mlir::Type ty,
mlir::ConversionPatternRewriter &rewriter) const {
assert(box.getType().isa<mlir::LLVM::LLVMPointerType>() &&
"descriptor inquiry with runtime dim can only be done on descriptor "
"in memory");
- auto pty = mlir::LLVM::LLVMPointerType::get(ty);
- mlir::LLVM::GEPOp p = genGEP(loc, pty, rewriter, box, 0,
+ mlir::LLVM::GEPOp p = genGEP(loc, boxTy.llvm, rewriter, box, 0,
static_cast<int>(kDimsPosInBox), dim, off);
auto loadOp = rewriter.create<mlir::LLVM::LoadOp>(loc, ty, p);
- attachTBAATag(loadOp, boxTy, nullptr, p);
+ attachTBAATag(loadOp, boxTy.fir, nullptr, p);
return loadOp;
}
mlir::Value
- getDimFieldFromBox(mlir::Location loc, mlir::Type boxTy, mlir::Value box,
+ getDimFieldFromBox(mlir::Location loc, TypePair boxTy, mlir::Value box,
int dim, int off, mlir::Type ty,
mlir::ConversionPatternRewriter &rewriter) const {
if (box.getType().isa<mlir::LLVM::LLVMPointerType>()) {
- auto pty = mlir::LLVM::LLVMPointerType::get(ty);
- mlir::LLVM::GEPOp p = genGEP(loc, pty, rewriter, box, 0,
+ mlir::LLVM::GEPOp p = genGEP(loc, boxTy.llvm, rewriter, box, 0,
static_cast<int>(kDimsPosInBox), dim, off);
auto loadOp = rewriter.create<mlir::LLVM::LoadOp>(loc, ty, p);
- attachTBAATag(loadOp, boxTy, nullptr, p);
+ attachTBAATag(loadOp, boxTy.fir, nullptr, p);
return loadOp;
}
return rewriter.create<mlir::LLVM::ExtractValueOp>(
@@ -246,7 +262,7 @@ class FIROpConversion : public mlir::ConvertOpToLLVMPattern<FromOp> {
}
mlir::Value
- getStrideFromBox(mlir::Location loc, mlir::Type boxTy, mlir::Value box,
+ getStrideFromBox(mlir::Location loc, TypePair boxTy, mlir::Value box,
unsigned dim,
mlir::ConversionPatternRewriter &rewriter) const {
auto idxTy = lowerTy().indexType();
@@ -256,26 +272,24 @@ class FIROpConversion : public mlir::ConvertOpToLLVMPattern<FromOp> {
/// Read base address from a fir.box. Returned address has type ty.
mlir::Value
- getBaseAddrFromBox(mlir::Location loc, mlir::Type resultTy, mlir::Type boxTy,
- mlir::Value box,
+ getBaseAddrFromBox(mlir::Location loc, TypePair boxTy, mlir::Value box,
mlir::ConversionPatternRewriter &rewriter) const {
+ mlir::Type resultTy = ::getLlvmPtrType(boxTy.llvm.getContext());
return getValueFromBox(loc, boxTy, box, resultTy, rewriter, kAddrPosInBox);
}
mlir::Value
- getElementSizeFromBox(mlir::Location loc, mlir::Type resultTy,
- mlir::Type boxTy, mlir::Value box,
+ getElementSizeFromBox(mlir::Location loc, mlir::Type resultTy, TypePair boxTy,
+ mlir::Value box,
mlir::ConversionPatternRewriter &rewriter) const {
return getValueFromBox(loc, boxTy, box, resultTy, rewriter,
kElemLenPosInBox);
}
// Get the element type given an LLVM type that is of the form
- // [llvm.ptr](array|struct|vector)+ and the provided indexes.
+ // (array|struct|vector)+ and the provided indexes.
static mlir::Type getBoxEleTy(mlir::Type type,
llvm::ArrayRef<std::int64_t> indexes) {
- if (auto t = type.dyn_cast<mlir::LLVM::LLVMPointerType>())
- type = t.getElementType();
for (unsigned i : indexes) {
if (auto t = type.dyn_cast<mlir::LLVM::LLVMStructType>()) {
assert(!t.isOpaque() && i < t.getBody().size());
@@ -292,17 +306,18 @@ class FIROpConversion : public mlir::ConvertOpToLLVMPattern<FromOp> {
return type;
}
- // Return LLVM type of the base address given the LLVM type
- // of the related descriptor (lowered fir.box type).
- static mlir::Type getBaseAddrTypeFromBox(mlir::Type type) {
- return getBoxEleTy(type, {kAddrPosInBox});
+ // Return LLVM type of the object described by a fir.box of \p boxType.
+ mlir::Type getLlvmObjectTypeFromBoxType(mlir::Type boxType) const {
+ mlir::Type objectType = fir::dyn_cast_ptrOrBoxEleTy(boxType);
+ assert(objectType && "boxType must be a box type");
+ return this->convertType(objectType);
}
/// Read the address of the type descriptor from a box.
mlir::Value
- loadTypeDescAddress(mlir::Location loc, mlir::Type boxTy, mlir::Value box,
+ loadTypeDescAddress(mlir::Location loc, TypePair boxTy, mlir::Value box,
mlir::ConversionPatternRewriter &rewriter) const {
- unsigned typeDescFieldId = getTypeDescFieldId(boxTy);
+ unsigned typeDescFieldId = getTypeDescFieldId(boxTy.fir);
mlir::Type tdescType = lowerTy().convertTypeDescType(rewriter.getContext());
return getValueFromBox(loc, boxTy, box, tdescType, rewriter,
typeDescFieldId);
@@ -310,7 +325,7 @@ class FIROpConversion : public mlir::ConvertOpToLLVMPattern<FromOp> {
// Load the attribute from the \p box and perform a check against \p maskValue
// The final comparison is implemented as `(attribute & maskValue) != 0`.
- mlir::Value genBoxAttributeCheck(mlir::Location loc, mlir::Type boxTy,
+ mlir::Value genBoxAttributeCheck(mlir::Location loc, TypePair boxTy,
mlir::Value box,
mlir::ConversionPatternRewriter &rewriter,
unsigned maskValue) const {
@@ -331,7 +346,8 @@ class FIROpConversion : public mlir::ConvertOpToLLVMPattern<FromOp> {
mlir::ConversionPatternRewriter &rewriter,
mlir::Value base, ARGS... args) const {
llvm::SmallVector<mlir::LLVM::GEPArg> cv = {args...};
- return rewriter.create<mlir::LLVM::GEPOp>(loc, ty, base, cv);
+ auto llvmPtrTy = ::getLlvmPtrType(ty.getContext());
+ return rewriter.create<mlir::LLVM::GEPOp>(loc, llvmPtrTy, ty, base, cv);
}
// Find the Block in which the alloca should be inserted.
@@ -348,16 +364,19 @@ class FIROpConversion : public mlir::ConvertOpToLLVMPattern<FromOp> {
return getBlockForAllocaInsert(op->getParentOp());
}
- // Generate an alloca of size 1 and type \p toTy.
+ // Generate an alloca of size 1 for an object of type \p llvmObjectTy.
mlir::LLVM::AllocaOp
- genAllocaWithType(mlir::Location loc, mlir::Type toTy, unsigned alignment,
+ genAllocaWithType(mlir::Location loc, mlir::Type llvmObjectTy,
+ unsigned alignment,
mlir::ConversionPatternRewriter &rewriter) const {
auto thisPt = rewriter.saveInsertionPoint();
mlir::Operation *parentOp = rewriter.getInsertionBlock()->getParentOp();
mlir::Block *insertBlock = getBlockForAllocaInsert(parentOp);
rewriter.setInsertionPointToStart(insertBlock);
auto size = genI32Constant(loc, rewriter, 1);
- auto al = rewriter.create<mlir::LLVM::AllocaOp>(loc, toTy, size, alignment);
+ mlir::Type llvmPtrTy = ::getLlvmPtrType(llvmObjectTy.getContext());
+ auto al = rewriter.create<mlir::LLVM::AllocaOp>(
+ loc, llvmPtrTy, llvmObjectTy, size, alignment);
rewriter.restoreInsertionPoint(thisPt);
return al;
}
@@ -471,8 +490,8 @@ struct AllocaOpConversion : public FIROpConversion<fir::AllocaOp> {
mlir::Type ity = lowerTy().indexType();
unsigned i = 0;
mlir::Value size = genConstantIndex(loc, ity, rewriter, 1).getResult();
- mlir::Type ty = convertType(alloc.getType());
- mlir::Type resultTy = ty;
+ mlir::Type firObjType = fir::unwrapRefType(alloc.getType());
+ mlir::Type llvmObjectType = convertObjectType(firObjType);
if (alloc.hasLenParams()) {
unsigned end = alloc.numLenParams();
llvm::SmallVector<mlir::Value> lenParams;
@@ -482,7 +501,7 @@ struct AllocaOpConversion : public FIROpConversion<fir::AllocaOp> {
if (auto chrTy = scalarType.dyn_cast<fir::CharacterType>()) {
fir::CharacterType rawCharTy = fir::CharacterType::getUnknownLen(
chrTy.getContext(), chrTy.getFKind());
- ty = mlir::LLVM::LLVMPointerType::get(convertType(rawCharTy));
+ llvmObjectType = convertType(rawCharTy);
assert(end == 1);
size = integerCast(loc, rewriter, ity, lenParams[0]);
} else if (auto recTy = scalarType.dyn_cast<fir::RecordType>()) {
@@ -495,7 +514,7 @@ struct AllocaOpConversion : public FIROpConversion<fir::AllocaOp> {
auto call = rewriter.create<mlir::LLVM::CallOp>(
loc, ity, lenParams, llvm::ArrayRef<mlir::NamedAttribute>{attr});
size = call.getResult();
- ty = ::getVoidPtrType(alloc.getContext());
+ llvmObjectType = ::getI8Type(alloc.getContext());
} else {
return emitError(loc, "unexpected type ")
<< scalarType << " with type parameters";
@@ -509,15 +528,20 @@ struct AllocaOpConversion : public FIROpConversion<fir::AllocaOp> {
size = rewriter.create<mlir::LLVM::MulOp>(
loc, ity, size, integerCast(loc, rewriter, ity, operands[i]));
}
- if (ty == resultTy) {
- // Do not emit the bitcast if ty and resultTy are the same.
- rewriter.replaceOpWithNewOp<mlir::LLVM::AllocaOp>(alloc, ty, size,
- alloc->getAttrs());
- } else {
- auto al = rewriter.create<mlir::LLVM::AllocaOp>(loc, ty, size,
- alloc->getAttrs());
- rewriter.replaceOpWithNewOp<mlir::LLVM::BitcastOp>(alloc, resultTy, al);
- }
+ mlir::Type llvmPtrTy = ::getLlvmPtrType(alloc.getContext());
+ // NOTE: we used to pass alloc->getAttrs() in the builder for non opaque
+ // pointers! Only propagate pinned and bindc_name to help debugging, but
+ // this should have no functional purpose (and passing the operand segment
+ // attribute like before is certainly bad).
+ auto llvmAlloc = rewriter.create<mlir::LLVM::AllocaOp>(
+ loc, llvmPtrTy, llvmObjectType, size);
+ if (alloc.getPinned())
+ llvmAlloc->setDiscardableAttr(alloc.getPinnedAttrName(),
+ alloc.getPinnedAttr());
+ if (alloc.getBindcName())
+ llvmAlloc->setDiscardableAttr(alloc.getBindcNameAttrName(),
+ alloc.getBindcNameAttr());
+ rewriter.replaceOp(alloc, llvmAlloc);
return mlir::success();
}
};
@@ -534,10 +558,10 @@ struct BoxAddrOpConversion : public FIROpConversion<fir::BoxAddrOp> {
mlir::ConversionPatternRewriter &rewriter) const override {
mlir::Value a = adaptor.getOperands()[0];
auto loc = boxaddr.getLoc();
- mlir::Type ty = convertType(boxaddr.getType());
if (auto argty = boxaddr.getVal().getType().dyn_cast<fir::BaseBoxType>()) {
+ TypePair boxTyPair = getBoxTypePair(argty);
rewriter.replaceOp(boxaddr,
- getBaseAddrFromBox(loc, ty, argty, a, rewriter));
+ getBaseAddrFromBox(loc, boxTyPair, a, rewriter));
} else {
rewriter.replaceOpWithNewOp<mlir::LLVM::ExtractValueOp>(boxaddr, a, 0);
}
@@ -581,9 +605,10 @@ struct BoxDimsOpConversion : public FIROpConversion<fir::BoxDimsOp> {
convertType(boxdims.getResult(1).getType()),
convertType(boxdims.getResult(2).getType()),
};
- auto results = getDimsFromBox(
- boxdims.getLoc(), resultTypes, boxdims.getVal().getType(),
- adaptor.getOperands()[0], adaptor.getOperands()[1], rewriter);
+ TypePair boxTyPair = getBoxTypePair(boxdims.getVal().getType());
+ auto results = getDimsFromBox(boxdims.getLoc(), resultTypes, boxTyPair,
+ adaptor.getOperands()[0],
+ adaptor.getOperands()[1], rewriter);
rewriter.replaceOp(boxdims, results);
return mlir::success();
}
@@ -600,8 +625,8 @@ struct BoxEleSizeOpConversion : public FIROpConversion<fir::BoxEleSizeOp> {
mlir::Value box = adaptor.getOperands()[0];
auto loc = boxelesz.getLoc();
auto ty = convertType(boxelesz.getType());
- auto elemSize = getElementSizeFromBox(loc, ty, boxelesz.getVal().getType(),
- box, rewriter);
+ TypePair boxTyPair = getBoxTypePair(boxelesz.getVal().getType());
+ auto elemSize = getElementSizeFromBox(loc, ty, boxTyPair, box, rewriter);
rewriter.replaceOp(boxelesz, elemSize);
return mlir::success();
}
@@ -617,8 +642,9 @@ struct BoxIsAllocOpConversion : public FIROpConversion<fir::BoxIsAllocOp> {
mlir::ConversionPatternRewriter &rewriter) const override {
mlir::Value box = adaptor.getOperands()[0];
auto loc = boxisalloc.getLoc();
- mlir::Value check = genBoxAttributeCheck(loc, boxisalloc.getVal().getType(),
- box, rewriter, kAttrAllocatable);
+ TypePair boxTyPair = getBoxTypePair(boxisalloc.getVal().getType());
+ mlir::Value check =
+ genBoxAttributeCheck(loc, boxTyPair, box, rewriter, kAttrAllocatable);
rewriter.replaceOp(boxisalloc, check);
return mlir::success();
}
@@ -634,8 +660,9 @@ struct BoxIsArrayOpConversion : public FIROpConversion<fir::BoxIsArrayOp> {
mlir::ConversionPatternRewriter &rewriter) const override {
mlir::Value a = adaptor.getOperands()[0];
auto loc = boxisarray.getLoc();
- auto rank = getValueFromBox(loc, boxisarray.getVal().getType(), a,
- rewriter.getI32Type(), rewriter, kRankPosInBox);
+ TypePair boxTyPair = getBoxTypePair(boxisarray.getVal().getType());
+ auto rank = getValueFromBox(loc, boxTyPair, a, rewriter.getI32Type(),
+ rewriter, kRankPosInBox);
auto c0 = genC...
[truncated]
|
This was referenced Oct 20, 2023
zero9178
reviewed
Oct 20, 2023
There was a problem hiding this comment.
Thank you so much for doing this and the incredible turnaround time!
I don't have any complaints regardings LLVM Dialect/opaque pointer usage and I am happy to see that the PR ends up being more red than green 🙂
I am not really qualified to comment on Flang specifics things so I'll leave this to other reviewers.
I did leave two comments about things that confused me as a flang outsider that I am curious about whether it is correct or whether I am simply missing some required context.
jeanPerier
commented
Oct 20, 2023
There was a problem hiding this comment.
Thanks a lot for the review! You pointed the finger at a type conversion of a fir type that is abstract and does not really match an llvm type on its own (but we still need to convert it to something for when it appears inside memory reference types or as operation attributes).
Thank you for the explanations! |
!llvm.ptr<T> typed pointers are depreciated in MLIR LLVM dialects. Flang codegen still generated them and relied on mlir.llvm codegen to LLVM to turn them into opaque pointers. This patch update FIR codegen to directly emit and work with LLVM type pointers. - All places generating GEPs need to add an extra type argument with the base type (the T that was previously in the llvm.ptr<T> of the base). - llvm.alloca must also be provided the object type. In the process, I doscovered that we were shamelessly copying all the attribute from fir.alloca to the llvm.alloca, which makes no sense for the operand segments. The updated code that cannot take an attribute dictionnary in the llvm.alloca builder with opaque pointers only propagate the "pinned" and "bindc_name" attributes to help debugging the generated IR. - Updating all the places that rely on getting the llvm object type from lowered llvm.ptr<T> arguments to get it from a type conversion of the original fir types. - Updating all the places that were generating llvm.ptr<T> types to generate the opaque llvm.ptr type. - Updating all the codegen tests checking generated MLIR llvm dialect. Many tests are testing directly LLVM IR, and this change is a no-op for those (which is expected).
jeanPerier
force-pushed
the
jpr-opaque-pointer-2
branch
from
26fd3c9
to
ba441f9
Compare
|
I rebased to incorporate #69772 (thanks @kiranchandramohan) and fix the conflicts. |
kiranchandramohan
approved these changes
Oct 24, 2023
clementval
approved these changes
Oct 24, 2023
Co-authored-by: Valentin Clement (バレンタイン クレメン) <clementval@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
!llvm.ptr typed pointers are depreciated in MLIR LLVM dialects. Flang codegen still generated them and relied on mlir.llvm codegen to LLVM to turn them into opaque pointers.
This patch update FIR codegen to directly emit and work with LLVM opaque pointers.
Addresses #69303
@kiranchandramohan and @TIFitis, this patch cannot be merged currently because the MLIR OpenMP dialect OpenMPToLLVMIRTranslation codegen pass relies on getting typed pointers (* see 1 below) and needs to be updated first.
But this patch is still submitted for review to help the OpenMP dialect update. See failing flang test OpenMP/FIR/function-filtering.f90.
All places generating GEPs need to add an extra type argument with the base type (the T that was previously in the llvm.ptr of the base).
llvm.alloca must also be provided the object type. In the process, I doscovered that we were shamelessly copying all the attribute from fir.alloca to the llvm.alloca, which makes no sense for the operand segments. The updated code that cannot take an attribute dictionnary in the llvm.alloca builder with opaque pointers only propagate the "pinned" and "bindc_name" attributes to help debugging the generated IR.
Updating all the places that rely on getting the llvm object type from lowered llvm.ptr arguments to get it from a type conversion of the original fir types.
Updating all the places that were generating llvm.ptr types to generate the opaque llvm.ptr type.
Updating all the codegen tests checking generated MLIR llvm dialect. Many tests are testing directly LLVM IR, and this change is a no-op for those (which is expected).