[flang-commits] [flang] e73cf2f - [flang] Remove materialization workaround in type converter (#98743)
via flang-commits
flang-commits at lists.llvm.org
Mon Jul 15 07:07:51 PDT 2024
Author: Matthias Springer
Date: 2024-07-15T16:07:48+02:00
New Revision: e73cf2f0c5b437f944e9a796d96d550d6ae3d8cf
URL: https://github.com/llvm/llvm-project/commit/e73cf2f0c5b437f944e9a796d96d550d6ae3d8cf
DIFF: https://github.com/llvm/llvm-project/commit/e73cf2f0c5b437f944e9a796d96d550d6ae3d8cf.diff
LOG: [flang] Remove materialization workaround in type converter (#98743)
This change is in preparation of #97903, which adds extra checks for
materializations: it is now enforced that they produce an SSA value of
the correct type, so the current workaround no longer works.
The original workaround avoided target materializations by directly
returning the to-be-converted SSA value from the materialization
callback. This can be avoided by initializing the lowering patterns that
insert the materializations without a type converter. For
`cg::XEmboxOp`, the existing workaround that skips
`unrealized_conversion_cast` ops is still in place.
Also remove the lowering pattern for `unrealized_conversion_cast`. This
pattern has no effect because `unrealized_conversion_cast` ops that are
inserted by the dialect conversion framework are never matched by the
pattern driver.
Added:
Modified:
flang/include/flang/Tools/CLOptions.inc
flang/lib/Optimizer/CodeGen/CodeGen.cpp
flang/lib/Optimizer/CodeGen/TypeConverter.cpp
flang/test/Fir/basic-program.fir
Removed:
################################################################################
diff --git a/flang/include/flang/Tools/CLOptions.inc b/flang/include/flang/Tools/CLOptions.inc
index 7f2910c5cfd3c..7df5044949463 100644
--- a/flang/include/flang/Tools/CLOptions.inc
+++ b/flang/include/flang/Tools/CLOptions.inc
@@ -9,6 +9,7 @@
/// This file defines some shared command-line options that can be used when
/// debugging the test tools. This file must be included into the tool.
+#include "mlir/Conversion/ReconcileUnrealizedCasts/ReconcileUnrealizedCasts.h"
#include "mlir/Conversion/SCFToControlFlow/SCFToControlFlow.h"
#include "mlir/Dialect/LLVMIR/LLVMAttrs.h"
#include "mlir/Pass/PassManager.h"
@@ -223,6 +224,10 @@ inline void addFIRToLLVMPass(
options.forceUnifiedTBAATree = useOldAliasTags;
addPassConditionally(pm, disableFirToLlvmIr,
[&]() { return fir::createFIRToLLVMPass(options); });
+ // The dialect conversion framework may leave dead unrealized_conversion_cast
+ // ops behind, so run reconcile-unrealized-casts to clean them up.
+ addPassConditionally(pm, disableFirToLlvmIr,
+ [&]() { return mlir::createReconcileUnrealizedCastsPass(); });
}
inline void addLLVMDialectToLLVMPass(
diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
index 7483acfcd1ca7..11535f073c807 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
@@ -35,7 +35,6 @@
#include "mlir/Conversion/MathToLLVM/MathToLLVM.h"
#include "mlir/Conversion/MathToLibm/MathToLibm.h"
#include "mlir/Conversion/OpenMPToLLVM/ConvertOpenMPToLLVM.h"
-#include "mlir/Conversion/ReconcileUnrealizedCasts/ReconcileUnrealizedCasts.h"
#include "mlir/Conversion/VectorToLLVM/ConvertVectorToLLVM.h"
#include "mlir/Dialect/Arith/IR/Arith.h"
#include "mlir/Dialect/DLTI/DLTI.h"
@@ -2042,13 +2041,13 @@ struct ExtractValueOpConversion
/// InsertValue is the generalized instruction for the composition of new
/// aggregate type values.
struct InsertValueOpConversion
- : public fir::FIROpAndTypeConversion<fir::InsertValueOp>,
+ : public mlir::OpConversionPattern<fir::InsertValueOp>,
public ValueOpCommon {
- using FIROpAndTypeConversion::FIROpAndTypeConversion;
+ using OpConversionPattern::OpConversionPattern;
llvm::LogicalResult
- doRewrite(fir::InsertValueOp insertVal, mlir::Type ty, OpAdaptor adaptor,
- mlir::ConversionPatternRewriter &rewriter) const override {
+ matchAndRewrite(fir::InsertValueOp insertVal, OpAdaptor adaptor,
+ mlir::ConversionPatternRewriter &rewriter) const override {
mlir::ValueRange operands = adaptor.getOperands();
auto indices = collectIndices(rewriter, insertVal.getCoor());
toRowMajor(indices, operands[0].getType());
@@ -2669,8 +2668,9 @@ struct TypeDescOpConversion : public fir::FIROpConversion<fir::TypeDescOp> {
};
/// Lower `fir.has_value` operation to `llvm.return` operation.
-struct HasValueOpConversion : public fir::FIROpConversion<fir::HasValueOp> {
- using FIROpConversion::FIROpConversion;
+struct HasValueOpConversion
+ : public mlir::OpConversionPattern<fir::HasValueOp> {
+ using OpConversionPattern::OpConversionPattern;
llvm::LogicalResult
matchAndRewrite(fir::HasValueOp op, OpAdaptor adaptor,
@@ -3515,29 +3515,6 @@ struct MustBeDeadConversion : public fir::FIROpConversion<FromOp> {
}
};
-struct UnrealizedConversionCastOpConversion
- : public fir::FIROpConversion<mlir::UnrealizedConversionCastOp> {
- using FIROpConversion::FIROpConversion;
-
- llvm::LogicalResult
- matchAndRewrite(mlir::UnrealizedConversionCastOp op, OpAdaptor adaptor,
- mlir::ConversionPatternRewriter &rewriter) const override {
- assert(op.getOutputs().getTypes().size() == 1 && "expect a single type");
- mlir::Type convertedType = convertType(op.getOutputs().getTypes()[0]);
- if (convertedType == adaptor.getInputs().getTypes()[0]) {
- rewriter.replaceOp(op, adaptor.getInputs());
- return mlir::success();
- }
-
- convertedType = adaptor.getInputs().getTypes()[0];
- if (convertedType == op.getOutputs().getType()[0]) {
- rewriter.replaceOp(op, adaptor.getInputs());
- return mlir::success();
- }
- return mlir::failure();
- }
-};
-
struct ShapeOpConversion : public MustBeDeadConversion<fir::ShapeOp> {
using MustBeDeadConversion::MustBeDeadConversion;
};
@@ -3714,7 +3691,8 @@ class FIRToLLVMLowering
signalPassFailure();
}
- // Run pass to add comdats to functions that have weak linkage on relevant platforms
+ // Run pass to add comdats to functions that have weak linkage on relevant
+ // platforms
if (fir::getTargetTriple(mod).supportsCOMDAT()) {
mlir::OpPassManager comdatPM("builtin.module");
comdatPM.addPass(mlir::LLVM::createLLVMAddComdats());
@@ -3789,16 +3767,19 @@ void fir::populateFIRToLLVMConversionPatterns(
DivcOpConversion, EmboxOpConversion, EmboxCharOpConversion,
EmboxProcOpConversion, ExtractValueOpConversion, FieldIndexOpConversion,
FirEndOpConversion, FreeMemOpConversion, GlobalLenOpConversion,
- GlobalOpConversion, HasValueOpConversion, InsertOnRangeOpConversion,
- InsertValueOpConversion, IsPresentOpConversion, LenParamIndexOpConversion,
- LoadOpConversion, MulcOpConversion, NegcOpConversion,
- NoReassocOpConversion, SelectCaseOpConversion, SelectOpConversion,
- SelectRankOpConversion, SelectTypeOpConversion, ShapeOpConversion,
- ShapeShiftOpConversion, ShiftOpConversion, SliceOpConversion,
- StoreOpConversion, StringLitOpConversion, SubcOpConversion,
- TypeDescOpConversion, TypeInfoOpConversion, UnboxCharOpConversion,
- UnboxProcOpConversion, UndefOpConversion, UnreachableOpConversion,
- UnrealizedConversionCastOpConversion, XArrayCoorOpConversion,
- XEmboxOpConversion, XReboxOpConversion, ZeroOpConversion>(converter,
- options);
+ GlobalOpConversion, InsertOnRangeOpConversion, IsPresentOpConversion,
+ LenParamIndexOpConversion, LoadOpConversion, MulcOpConversion,
+ NegcOpConversion, NoReassocOpConversion, SelectCaseOpConversion,
+ SelectOpConversion, SelectRankOpConversion, SelectTypeOpConversion,
+ ShapeOpConversion, ShapeShiftOpConversion, ShiftOpConversion,
+ SliceOpConversion, StoreOpConversion, StringLitOpConversion,
+ SubcOpConversion, TypeDescOpConversion, TypeInfoOpConversion,
+ UnboxCharOpConversion, UnboxProcOpConversion, UndefOpConversion,
+ UnreachableOpConversion, XArrayCoorOpConversion, XEmboxOpConversion,
+ XReboxOpConversion, ZeroOpConversion>(converter, options);
+
+ // Patterns that are populated without a type converter do not trigger
+ // target materializations for the operands of the root op.
+ patterns.insert<HasValueOpConversion, InsertValueOpConversion>(
+ patterns.getContext());
}
diff --git a/flang/lib/Optimizer/CodeGen/TypeConverter.cpp b/flang/lib/Optimizer/CodeGen/TypeConverter.cpp
index ce86c625e082f..7b46a1a92142b 100644
--- a/flang/lib/Optimizer/CodeGen/TypeConverter.cpp
+++ b/flang/lib/Optimizer/CodeGen/TypeConverter.cpp
@@ -122,40 +122,6 @@ LLVMTypeConverter::LLVMTypeConverter(mlir::ModuleOp module, bool applyTBAA,
// Convert it here to i1 just in case it survives.
return mlir::IntegerType::get(&getContext(), 1);
});
- // FIXME: https://reviews.llvm.org/D82831 introduced an automatic
- // materialization of conversion around function calls that is not working
- // well with fir lowering to llvm (incorrect llvm.mlir.cast are inserted).
- // Workaround until better analysis: register a handler that does not insert
- // any conversions.
- addSourceMaterialization(
- [&](mlir::OpBuilder &builder, mlir::Type resultType,
- mlir::ValueRange inputs,
- mlir::Location loc) -> std::optional<mlir::Value> {
- if (inputs.size() != 1)
- return std::nullopt;
- return inputs[0];
- });
- // Similar FIXME workaround here (needed for compare.fir/select-type.fir
- // as well as rebox-global.fir tests). This is needed to cope with the
- // the fact that codegen does not lower some operation results to the LLVM
- // type produced by this LLVMTypeConverter. For instance, inside FIR
- // globals, fir.box are lowered to llvm.struct, while the fir.box type
- // conversion translates it into an llvm.ptr<llvm.struct<>> because
- // descriptors are manipulated in memory outside of global initializers
- // where this is not possible. Hence, MLIR inserts
- // builtin.unrealized_conversion_cast after the translation of operations
- // producing fir.box in fir.global codegen. addSourceMaterialization and
- // addTargetMaterialization allow ignoring these ops and removing them
- // after codegen assuming the type discrepencies are intended (like for
- // fir.box inside globals).
- addTargetMaterialization(
- [&](mlir::OpBuilder &builder, mlir::Type resultType,
- mlir::ValueRange inputs,
- mlir::Location loc) -> std::optional<mlir::Value> {
- if (inputs.size() != 1)
- return std::nullopt;
- return inputs[0];
- });
}
// i32 is used here because LLVM wants i32 constants when indexing into struct
diff --git a/flang/test/Fir/basic-program.fir b/flang/test/Fir/basic-program.fir
index 7bbfd709b0aaf..dda4f32872fef 100644
--- a/flang/test/Fir/basic-program.fir
+++ b/flang/test/Fir/basic-program.fir
@@ -119,4 +119,5 @@ func.func @_QQmain() {
// PASSES-NEXT: (S) 0 num-dce'd - Number of operations eliminated
// PASSES-NEXT: TargetRewrite
// PASSES-NEXT: FIRToLLVMLowering
+// PASSES-NEXT: ReconcileUnrealizedCasts
// PASSES-NEXT: LLVMIRLoweringPass
More information about the flang-commits
mailing list