-
Notifications
You must be signed in to change notification settings - Fork 10.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[OpenMP][MLIR] OMPEarlyOutliningPass removal #67319
Conversation
@llvm/pr-subscribers-flang-driver @llvm/pr-subscribers-flang-fir-hlfir ChangesThis patch removes the OMPEarlyOutliningPass as it is no longer required. The implicit map operand capture has now been moved to the PFT lowering stage. Patch is 54.39 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/67319.diff 21 Files Affected:
diff --git a/flang/include/flang/Optimizer/Transforms/Passes.h b/flang/include/flang/Optimizer/Transforms/Passes.h
index 8aeb3e373298e88..460aeafbe1a7e19 100644
--- a/flang/include/flang/Optimizer/Transforms/Passes.h
+++ b/flang/include/flang/Optimizer/Transforms/Passes.h
@@ -74,8 +74,6 @@ std::unique_ptr<mlir::Pass>
createAlgebraicSimplificationPass(const mlir::GreedyRewriteConfig &config);
std::unique_ptr<mlir::Pass> createPolymorphicOpConversionPass();
-std::unique_ptr<mlir::OperationPass<mlir::ModuleOp>>
-createOMPEarlyOutliningPass();
std::unique_ptr<mlir::Pass> createOMPFunctionFilteringPass();
std::unique_ptr<mlir::OperationPass<mlir::ModuleOp>>
createOMPMarkDeclareTargetPass();
diff --git a/flang/include/flang/Optimizer/Transforms/Passes.td b/flang/include/flang/Optimizer/Transforms/Passes.td
index 9474edf13ce4639..dd4d99a1060af90 100644
--- a/flang/include/flang/Optimizer/Transforms/Passes.td
+++ b/flang/include/flang/Optimizer/Transforms/Passes.td
@@ -16,7 +16,7 @@
include "mlir/Pass/PassBase.td"
-class AbstractResultOptBase<string optExt, string operation>
+class AbstractResultOptBase<string optExt, string operation>
: Pass<"abstract-result-on-" # optExt # "-opt", operation> {
let summary = "Convert fir.array, fir.box and fir.rec function result to "
"function argument";
@@ -277,8 +277,8 @@ def PolymorphicOpConversion : Pass<"fir-polymorphic-op", "::mlir::func::FuncOp">
let summary =
"Simplify operations on polymorphic types";
let description = [{
- This pass breaks up the lowering of operations on polymorphic types by
- introducing an intermediate FIR level that simplifies code geneation.
+ This pass breaks up the lowering of operations on polymorphic types by
+ introducing an intermediate FIR level that simplifies code geneation.
}];
let constructor = "::fir::createPolymorphicOpConversionPass()";
let dependentDialects = [
@@ -298,18 +298,6 @@ def LoopVersioning : Pass<"loop-versioning", "mlir::func::FuncOp"> {
let dependentDialects = [ "fir::FIROpsDialect" ];
}
-def OMPEarlyOutliningPass
- : Pass<"omp-early-target-outlining", "mlir::ModuleOp"> {
- let summary = "Outlines all target ops into separate functions";
- let description = [{
- This pass outlines all omp.target operations into individual functions.
- It is invoked in the front end after the initial FIR has been constructed.
- This pass is only needed when compiling for the target device to prevent
- the optimizer to perform transforms across target region boundaries.
- }];
- let constructor = "::fir::createOMPEarlyOutliningPass()";
-}
-
def OMPMarkDeclareTargetPass
: Pass<"omp-mark-declare-target", "mlir::ModuleOp"> {
let summary = "Marks all functions called by an OpenMP declare target function as declare target";
diff --git a/flang/include/flang/Tools/CLOptions.inc b/flang/include/flang/Tools/CLOptions.inc
index 616d9ddc066a75d..5458eb3187a24e4 100644
--- a/flang/include/flang/Tools/CLOptions.inc
+++ b/flang/include/flang/Tools/CLOptions.inc
@@ -266,10 +266,8 @@ inline void createHLFIRToFIRPassPipeline(
inline void createOpenMPFIRPassPipeline(
mlir::PassManager &pm, bool isTargetDevice) {
pm.addPass(fir::createOMPMarkDeclareTargetPass());
- if (isTargetDevice) {
- pm.addPass(fir::createOMPEarlyOutliningPass());
+ if (isTargetDevice)
pm.addPass(fir::createOMPFunctionFilteringPass());
- }
}
#if !defined(FLANG_EXCLUDE_CODEGEN)
diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index 5f5e968eaaa6414..c369cba4255d4f4 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -538,7 +538,11 @@ class ClauseProcessor {
const llvm::omp::Directive &directive,
Fortran::semantics::SemanticsContext &semanticsContext,
Fortran::lower::StatementContext &stmtCtx,
- llvm::SmallVectorImpl<mlir::Value> &mapOperands) const;
+ llvm::SmallVectorImpl<mlir::Value> &mapOperands,
+ llvm::SmallVectorImpl<mlir::Type> *mapSymTypes = nullptr,
+ llvm::SmallVectorImpl<mlir::Location> *mapSymLocs = nullptr,
+ llvm::SmallVectorImpl<const Fortran::semantics::Symbol *>
+ *mapSymbols = nullptr) const;
bool processReduction(
mlir::Location currentLocation,
llvm::SmallVectorImpl<mlir::Value> &reductionVars,
@@ -1665,7 +1669,7 @@ static mlir::omp::MapInfoOp
createMapInfoOp(fir::FirOpBuilder &builder, mlir::Location loc,
mlir::Value baseAddr, std::stringstream &name,
mlir::SmallVector<mlir::Value> bounds, uint64_t mapType,
- mlir::omp::VariableCaptureKind mapCaptureType, bool implicit,
+ mlir::omp::VariableCaptureKind mapCaptureType,
mlir::Type retTy) {
mlir::Value varPtrPtr;
if (auto boxTy = baseAddr.getType().dyn_cast<fir::BaseBoxType>()) {
@@ -1676,7 +1680,6 @@ createMapInfoOp(fir::FirOpBuilder &builder, mlir::Location loc,
mlir::omp::MapInfoOp op =
builder.create<mlir::omp::MapInfoOp>(loc, retTy, baseAddr);
op.setNameAttr(builder.getStringAttr(name.str()));
- op.setImplicit(implicit);
op.setMapType(mapType);
op.setMapCaptureType(mapCaptureType);
@@ -1695,7 +1698,11 @@ bool ClauseProcessor::processMap(
mlir::Location currentLocation, const llvm::omp::Directive &directive,
Fortran::semantics::SemanticsContext &semanticsContext,
Fortran::lower::StatementContext &stmtCtx,
- llvm::SmallVectorImpl<mlir::Value> &mapOperands) const {
+ llvm::SmallVectorImpl<mlir::Value> &mapOperands,
+ llvm::SmallVectorImpl<mlir::Type> *mapSymTypes,
+ llvm::SmallVectorImpl<mlir::Location> *mapSymLocs,
+ llvm::SmallVectorImpl<const Fortran::semantics::Symbol *> *mapSymbols)
+ const {
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
return findRepeatableClause<ClauseTy::Map>(
[&](const ClauseTy::Map *mapClause,
@@ -1755,13 +1762,20 @@ bool ClauseProcessor::processMap(
// Explicit map captures are captured ByRef by default,
// optimisation passes may alter this to ByCopy or other capture
// types to optimise
- mapOperands.push_back(createMapInfoOp(
+ mlir::Value mapOp = createMapInfoOp(
firOpBuilder, clauseLocation, baseAddr, asFortran, bounds,
static_cast<
std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>(
mapTypeBits),
- mlir::omp::VariableCaptureKind::ByRef, false,
- baseAddr.getType()));
+ mlir::omp::VariableCaptureKind::ByRef, baseAddr.getType());
+
+ mapOperands.push_back(mapOp);
+ if (mapSymTypes)
+ mapSymTypes->push_back(mapOp.getType());
+ if (mapSymLocs)
+ mapSymLocs->push_back(mapOp.getLoc());
+ if (mapSymbols)
+ mapSymbols->push_back(getOmpObjectSymbol(ompObject));
}
});
}
@@ -2142,7 +2156,7 @@ static void createBodyOfOp(
}
}
-static void createBodyOfTargetDataOp(
+static void genBodyOfTargetDataOp(
Fortran::lower::AbstractConverter &converter, mlir::omp::DataOp &dataOp,
const llvm::SmallVector<mlir::Type> &useDeviceTypes,
const llvm::SmallVector<mlir::Location> &useDeviceLocs,
@@ -2356,8 +2370,8 @@ genDataOp(Fortran::lower::AbstractConverter &converter,
auto dataOp = converter.getFirOpBuilder().create<mlir::omp::DataOp>(
currentLocation, ifClauseOperand, deviceOperand, devicePtrOperands,
deviceAddrOperands, mapOperands);
- createBodyOfTargetDataOp(converter, dataOp, useDeviceTypes, useDeviceLocs,
- useDeviceSymbols, currentLocation);
+ genBodyOfTargetDataOp(converter, dataOp, useDeviceTypes, useDeviceLocs,
+ useDeviceSymbols, currentLocation);
return dataOp;
}
@@ -2400,6 +2414,52 @@ genEnterExitDataOp(Fortran::lower::AbstractConverter &converter,
deviceOperand, nowaitAttr, mapOperands);
}
+static void genBodyOfTargetOp(
+ Fortran::lower::AbstractConverter &converter, mlir::omp::TargetOp &targetOp,
+ const llvm::SmallVector<mlir::Type> &mapSymTypes,
+ const llvm::SmallVector<mlir::Location> &mapSymLocs,
+ const llvm::SmallVector<const Fortran::semantics::Symbol *> &mapSymbols,
+ const mlir::Location ¤tLocation) {
+ assert(mapSymTypes.size() == mapSymLocs.size() &&
+ mapSymTypes.size() == mapSymbols.size());
+
+ fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+ mlir::Region ®ion = targetOp.getRegion();
+
+ firOpBuilder.createBlock(®ion, {}, mapSymTypes, mapSymLocs);
+ firOpBuilder.create<mlir::omp::TerminatorOp>(currentLocation);
+ firOpBuilder.setInsertionPointToStart(®ion.front());
+
+ unsigned argIndex = 0;
+ for (const Fortran::semantics::Symbol *sym : mapSymbols) {
+ const mlir::BlockArgument &arg = region.front().getArgument(argIndex);
+ fir::ExtendedValue extVal = converter.getSymbolExtendedValue(*sym);
+ mlir::Value val = fir::getBase(arg);
+ extVal.match(
+ [&](const fir::BoxValue &v) {
+ converter.bindSymbol(*sym, fir::BoxValue(val, v.getLBounds(),
+ v.getExplicitParameters(),
+ v.getExplicitExtents()));
+ },
+ [&](const fir::MutableBoxValue &v) {
+ converter.bindSymbol(*sym,
+ fir::MutableBoxValue(val, v.getLBounds(),
+ v.getMutableProperties()));
+ },
+ [&](const fir::ArrayBoxValue &v) {
+ converter.bindSymbol(*sym, fir::ArrayBoxValue(val, v.getExtents(),
+ v.getLBounds(),
+ v.getSourceBox()));
+ },
+ [&](const fir::UnboxedValue &v) { converter.bindSymbol(*sym, val); },
+ [&](const auto &) {
+ TODO(converter.getCurrentLocation(),
+ "target map clause operand unsupported type");
+ });
+ argIndex++;
+ }
+}
+
static mlir::omp::TargetOp
genTargetOp(Fortran::lower::AbstractConverter &converter,
Fortran::lower::pft::Evaluation &eval,
@@ -2411,6 +2471,9 @@ genTargetOp(Fortran::lower::AbstractConverter &converter,
mlir::Value ifClauseOperand, deviceOperand, threadLimitOperand;
mlir::UnitAttr nowaitAttr;
llvm::SmallVector<mlir::Value> mapOperands;
+ llvm::SmallVector<mlir::Type> mapSymTypes;
+ llvm::SmallVector<mlir::Location> mapSymLocs;
+ llvm::SmallVector<const Fortran::semantics::Symbol *> mapSymbols;
ClauseProcessor cp(converter, clauseList);
cp.processIf(stmtCtx,
@@ -2420,7 +2483,7 @@ genTargetOp(Fortran::lower::AbstractConverter &converter,
cp.processThreadLimit(stmtCtx, threadLimitOperand);
cp.processNowait(nowaitAttr);
cp.processMap(currentLocation, directive, semanticsContext, stmtCtx,
- mapOperands);
+ mapOperands, &mapSymTypes, &mapSymLocs, &mapSymbols);
cp.processTODO<Fortran::parser::OmpClause::Private,
Fortran::parser::OmpClause::Depend,
Fortran::parser::OmpClause::Firstprivate,
@@ -2433,10 +2496,44 @@ genTargetOp(Fortran::lower::AbstractConverter &converter,
Fortran::parser::OmpClause::Defaultmap>(
currentLocation, llvm::omp::Directive::OMPD_target);
- return genOpWithBody<mlir::omp::TargetOp>(
- converter, eval, currentLocation, outerCombined, &clauseList,
- ifClauseOperand, deviceOperand, threadLimitOperand, nowaitAttr,
- mapOperands);
+ auto captureImplicitMap = [&](const Fortran::semantics::Symbol &sym) {
+ if (llvm::find(mapSymbols, &sym) == mapSymbols.end()) {
+ mlir::Value baseOp = converter.getSymbolAddress(sym);
+ if (!baseOp)
+ if (const auto *details = sym.template detailsIf<
+ Fortran::semantics::HostAssocDetails>()) {
+ baseOp = converter.getSymbolAddress(details->symbol());
+ converter.copySymbolBinding(details->symbol(), sym);
+ }
+
+ if (baseOp) {
+ llvm::SmallVector<mlir::Value> bounds;
+ std::stringstream name;
+ name << sym.name().ToString();
+ mlir::Value mapOp = createMapInfoOp(
+ converter.getFirOpBuilder(), baseOp.getLoc(), baseOp, name, bounds,
+ static_cast<
+ std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>(
+ llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_LITERAL |
+ llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_IMPLICIT),
+ mlir::omp::VariableCaptureKind::ByCopy, baseOp.getType());
+ mapOperands.push_back(mapOp);
+ mapSymTypes.push_back(baseOp.getType());
+ mapSymLocs.push_back(baseOp.getLoc());
+ mapSymbols.push_back(&sym);
+ }
+ }
+ };
+ Fortran::lower::pft::visitAllSymbols(eval, captureImplicitMap);
+
+ auto targetOp = converter.getFirOpBuilder().create<mlir::omp::TargetOp>(
+ currentLocation, ifClauseOperand, deviceOperand, threadLimitOperand,
+ nowaitAttr, mapOperands);
+
+ genBodyOfTargetOp(converter, targetOp, mapSymTypes, mapSymLocs, mapSymbols,
+ currentLocation);
+
+ return targetOp;
}
static mlir::omp::TeamsOp
diff --git a/flang/lib/Optimizer/Transforms/CMakeLists.txt b/flang/lib/Optimizer/Transforms/CMakeLists.txt
index 3d2b7e5eaeade0a..4035fa9908a7778 100644
--- a/flang/lib/Optimizer/Transforms/CMakeLists.txt
+++ b/flang/lib/Optimizer/Transforms/CMakeLists.txt
@@ -16,7 +16,6 @@ add_flang_library(FIRTransforms
AddDebugFoundation.cpp
PolymorphicOpConversion.cpp
LoopVersioning.cpp
- OMPEarlyOutlining.cpp
OMPFunctionFiltering.cpp
OMPMarkDeclareTarget.cpp
diff --git a/flang/lib/Optimizer/Transforms/OMPEarlyOutlining.cpp b/flang/lib/Optimizer/Transforms/OMPEarlyOutlining.cpp
deleted file mode 100644
index 355a687d5f88588..000000000000000
--- a/flang/lib/Optimizer/Transforms/OMPEarlyOutlining.cpp
+++ /dev/null
@@ -1,261 +0,0 @@
-#include "flang/Optimizer/Dialect/FIRDialect.h"
-#include "flang/Optimizer/Dialect/FIROps.h"
-#include "flang/Optimizer/Dialect/FIRType.h"
-#include "flang/Optimizer/Support/InternalNames.h"
-#include "flang/Optimizer/Transforms/Passes.h"
-#include "mlir/Dialect/Func/IR/FuncOps.h"
-#include "mlir/Dialect/LLVMIR/LLVMDialect.h"
-#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
-#include "mlir/IR/BuiltinDialect.h"
-#include "mlir/IR/BuiltinOps.h"
-#include "mlir/IR/IRMapping.h"
-#include "mlir/IR/Operation.h"
-#include "mlir/IR/SymbolTable.h"
-#include "mlir/Pass/Pass.h"
-#include "mlir/Support/LLVM.h"
-#include "mlir/Transforms/RegionUtils.h"
-#include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
-
-namespace fir {
-#define GEN_PASS_DEF_OMPEARLYOUTLININGPASS
-#include "flang/Optimizer/Transforms/Passes.h.inc"
-} // namespace fir
-
-namespace {
-class OMPEarlyOutliningPass
- : public fir::impl::OMPEarlyOutliningPassBase<OMPEarlyOutliningPass> {
-
- std::string getOutlinedFnName(llvm::StringRef parentName, unsigned count) {
- return std::string(parentName) + "_omp_outline_" + std::to_string(count);
- }
-
- // Given a value this function will iterate over an operators results
- // and return the relevant index for the result the value corresponds to.
- // There may be a simpler way to do this however.
- static unsigned getResultIndex(mlir::Value value, mlir::Operation *op) {
- for (unsigned i = 0; i < op->getNumResults(); ++i) {
- if (op->getResult(i) == value)
- return i;
- }
- return 0;
- }
-
- static bool isAddressOfGlobalDeclareTarget(mlir::Value value) {
- if (fir::AddrOfOp addressOfOp =
- mlir::dyn_cast_if_present<fir::AddrOfOp>(value.getDefiningOp()))
- if (fir::GlobalOp gOp = mlir::dyn_cast_if_present<fir::GlobalOp>(
- addressOfOp->getParentOfType<mlir::ModuleOp>().lookupSymbol(
- addressOfOp.getSymbol())))
- if (auto declareTargetGlobal =
- llvm::dyn_cast<mlir::omp::DeclareTargetInterface>(
- gOp.getOperation()))
- if (declareTargetGlobal.isDeclareTarget())
- return true;
- return false;
- }
-
- // Currently used for cloning arguments that are nested. Should be
- // extendable where required, perhaps via operation
- // specialisation/overloading, if something needs specialised handling.
- // NOTE: Results in duplication of some values that would otherwise be
- // a single SSA value shared between operations, this is tidied up on
- // lowering to some extent.
- static mlir::Operation *
- cloneArgAndChildren(mlir::OpBuilder &builder, mlir::Operation *op,
- llvm::SetVector<mlir::Value> &inputs,
- mlir::Block::BlockArgListType &newInputs) {
- mlir::IRMapping valueMap;
- for (mlir::Value opValue : op->getOperands()) {
- if (opValue.getDefiningOp()) {
- unsigned resIdx = getResultIndex(opValue, opValue.getDefiningOp());
- valueMap.map(opValue,
- cloneArgAndChildren(builder, opValue.getDefiningOp(),
- inputs, newInputs)
- ->getResult(resIdx));
- } else {
- for (auto inArg : llvm::zip(inputs, newInputs)) {
- if (opValue == std::get<0>(inArg))
- valueMap.map(opValue, std::get<1>(inArg));
- }
- }
- }
-
- return builder.clone(*op, valueMap);
- }
-
- static void cloneMapOpVariables(mlir::OpBuilder &builder,
- mlir::IRMapping &valueMap,
- mlir::IRMapping &mapInfoMap,
- llvm::SetVector<mlir::Value> &inputs,
- mlir::Block::BlockArgListType &newInputs,
- mlir::Value varPtr) {
- if (fir::BoxAddrOp boxAddrOp =
- mlir::dyn_cast_if_present<fir::BoxAddrOp>(varPtr.getDefiningOp())) {
- mlir::Value newV =
- cloneArgAndChildren(builder, boxAddrOp, inputs, newInputs)
- ->getResult(0);
- mapInfoMap.map(varPtr, newV);
- valueMap.map(boxAddrOp, newV);
- return;
- }
-
- if (isAddressOfGlobalDeclareTarget(varPtr)) {
- fir::AddrOfOp addrOp =
- mlir::dyn_cast<fir::AddrOfOp>(varPtr.getDefiningOp());
- mlir::Value newV = builder.clone(*addrOp)->getResult(0);
- mapInfoMap.map(varPtr, newV);
- valueMap.map(addrOp, newV);
- return;
- }
-
- for (auto inArg : llvm::zip(inputs, newInputs)) {
- if (varPtr == std::get<0>(inArg))
- mapInfoMap.map(varPtr, std::get<1>(inArg));
- }
- }
-
- mlir::func::FuncOp outlineTargetOp(mlir::OpBuilder &builder,
- mlir::omp::TargetOp &targetOp,
- mlir::func::FuncOp &parentFunc,
- unsigned count) {
- // NOTE: once implicit captures are handled appropriately in the initial
- // PFT lowering if it is possible, we can remove the usage of
- // getUsedValuesDefinedAbove and instead just iterate over the target op's
- // operands (or just the map arguments) and perhaps refactor this function
- // a little.
- // Collect inputs
- llvm::SetVector<mlir::Value> inputs;
- mlir::Region &targetRegion = targetOp.getRegion();
- mlir::getUsedValuesDefinedAbove(targetRegion, inputs);
-
- // filter out declareTarget and map entries which are specially handled
- // at the moment, so we do not wish these to end up as function arguments
- // which would just be more noise in the IR.
- for (llvm::SetVector<mlir::Value>::iterator iter = inputs.begin(); iter != inputs.end();) {
- if (mlir::isa_and_nonnull<mlir::omp::MapInfoOp>(iter->getDefiningOp()) ||
- isAddressOfGlobalDeclareTarget(*iter)) {
- iter = inputs.erase(iter);
- } else {
- ++iter;
- }
- }
-
- // Create new function and initialize
- mlir::FunctionType funcType = builder.getFunctionType(
- mlir::TypeRange(inputs.getArrayRef()), mlir::TypeRange());
- std::string parentName(parentFunc.getName());
- std::string funcName = getOutline...
[truncated]
|
This change seems to break function filtering in certain cases. For example: ! File: test.f90
program main
!$omp target
!$omp end target
print *, 5
end program main This results in a compiler crash around The target outlining pass allowed the function filtering pass to assume that the only MLIR operations that remain represent either device code or wrapper functions around a single |
@skatrak Thanks for pointing this out, I'm working on a fix now. |
0cae145
to
22faa9f
Compare
0ca06b1
to
af1c33f
Compare
56abd2a
to
4011066
Compare
✅ With the latest revision this PR passed the C/C++ code formatter. |
4011066
to
8d22aad
Compare
The patch series LGTM so far, the only concern I have (which is very minor) is the map type and capture type we give the bounds information when we generate map_info's for them, but I think that's something we can adjust in future patches if we have any problems as I am not sure myself what they should be mapped as currently. Provided this patch series doesn't break any existing runtime tests we have I'm happy with it. I've also added one comment relating to manipulating the capture type of implicits based on how Clang handles it currently, we can add it in this patch series or later I don't mind too much, it just seemed like a good possible addition. |
8ffc572
to
412629c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG. Please wait for @agozillon.
412629c
to
84f2c01
Compare
f03b2f6
to
efb4288
Compare
llvmFunc->replaceAllUsesWith( | ||
llvm::UndefValue::get(llvmFunc->getType())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@skatrak I have add this line to replace the uses of the function that are to be removed with undef
values. This should prevent the crash.
Let me know if this looks like an appropriate fix for the issue to you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a good solution to the problem. Thanks!
efb4288
to
7fd28e7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, although, please do wait on @skatrak input on the fix you've pinged him in, it looks reasonable to me, but he will understand the implications of the change a lot better than I do. And thank you for your hard work on this amazing series of PRs.
This patch removes the OMPEarlyOutliningPass as it is no longer required. The implicit map operand capture has now been moved to the PFT lowering stage. Depends on llvm#67318.
7fd28e7
to
77a3538
Compare
target_map_common_block2.f90 - Fix the extra space in the print message. - llvm#67164 fixes this. So moving it outside of failing and also removing XFAIL marker. basic-target-region-3D-array.f90 - Corrected the check to account for the new lines printed. Depends on llvm#67319
Pushed commit 6375239. |
After the removal of the OpenMP early outlining MLIR pass in llvm#67319, the `EarlyOutliningInterface` stopped doing any useful work. It used to be necessary to tie the name of the function from which a target region was outlined to that new function, so it would be used when translating to LLVM IR in place of the outlined function's name. This is not necessary anymore, so this patch removes all references to this interface and uses of the `omp.outline_parent_name` discardable attribute in tests.
After the removal of the OpenMP early outlining MLIR pass in #67319, the `EarlyOutliningInterface` stopped doing any useful work. It used to be necessary to tie the name of the function from which a target region was outlined to that new function, so it would be used when translating to LLVM IR in place of the outlined function's name. This is not necessary anymore, so this patch removes all references to this interface and uses of the `omp.outline_parent_name` discardable attribute in tests.
This patch removes the OMPEarlyOutliningPass as it is no longer required. The implicit map operand capture has now been moved to the PFT lowering stage.
Depends on #67318.