-
Notifications
You must be signed in to change notification settings - Fork 14.7k
[flang][OpenMP] Add -f[no]-openmp-simd #150269
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
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-flang-semantics @llvm/pr-subscribers-clang-driver Author: Kajetan Puchalski (mrkajetanp) ChangesBoth clang and gfortran support the -fopenmp-simd flag, which enables OpenMP support only for simd constructs, while disabling the rest of OpenMP. Add a new SimdOnly flang OpenMP IR pass which rewrites generated OpenMP FIR to remove all constructs except for omp.simd constructs, and constructs nested under them. The flag is expected to have no effect if -fopenmp is passed explicitly, and is only expected to remove OpenMP constructs, not things like OpenMP library functions calls. This matches the behaviour of other compilers. Patch is 70.32 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/150269.diff 18 Files Affected:
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 916400efdb449..7a74dcffde4a9 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3706,14 +3706,19 @@ def fopenmp_relocatable_target : Flag<["-"], "fopenmp-relocatable-target">,
def fnoopenmp_relocatable_target : Flag<["-"], "fnoopenmp-relocatable-target">,
Group<f_Group>, Flags<[NoArgumentUnused, HelpHidden]>,
Visibility<[ClangOption, CC1Option]>;
-def fopenmp_simd : Flag<["-"], "fopenmp-simd">, Group<f_Group>,
- Flags<[NoArgumentUnused]>, Visibility<[ClangOption, CC1Option]>,
- HelpText<"Emit OpenMP code only for SIMD-based constructs.">;
+def fopenmp_simd : Flag<["-"], "fopenmp-simd">,
+ Group<f_Group>,
+ Flags<[NoArgumentUnused]>,
+ Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>,
+ HelpText<"Emit OpenMP code only for SIMD-based constructs.">;
def fopenmp_enable_irbuilder : Flag<["-"], "fopenmp-enable-irbuilder">, Group<f_Group>,
Flags<[NoArgumentUnused, HelpHidden]>, Visibility<[ClangOption, CC1Option]>,
HelpText<"Use the experimental OpenMP-IR-Builder codegen path.">;
-def fno_openmp_simd : Flag<["-"], "fno-openmp-simd">, Group<f_Group>,
- Flags<[NoArgumentUnused]>, Visibility<[ClangOption, CC1Option]>;
+def fno_openmp_simd
+ : Flag<["-"], "fno-openmp-simd">,
+ Group<f_Group>,
+ Flags<[NoArgumentUnused]>,
+ Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>;
def fopenmp_cuda_mode : Flag<["-"], "fopenmp-cuda-mode">, Group<f_Group>,
Flags<[NoArgumentUnused, HelpHidden]>, Visibility<[ClangOption, CC1Option]>;
def fno_openmp_cuda_mode : Flag<["-"], "fno-openmp-cuda-mode">, Group<f_Group>,
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index 7ab41e9b85a04..547e3156f519a 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -937,6 +937,8 @@ void Flang::ConstructJob(Compilation &C, const JobAction &JA,
if (Args.hasArg(options::OPT_fopenmp_force_usm))
CmdArgs.push_back("-fopenmp-force-usm");
+ Args.AddLastArg(CmdArgs, options::OPT_fopenmp_simd,
+ options::OPT_fno_openmp_simd);
// FIXME: Clang supports a whole bunch more flags here.
break;
@@ -952,6 +954,9 @@ void Flang::ConstructJob(Compilation &C, const JobAction &JA,
<< A->getSpelling() << A->getValue();
break;
}
+ } else {
+ Args.AddLastArg(CmdArgs, options::OPT_fopenmp_simd,
+ options::OPT_fno_openmp_simd);
}
// Pass the path to compiler resource files.
diff --git a/flang/include/flang/Optimizer/OpenMP/Passes.td b/flang/include/flang/Optimizer/OpenMP/Passes.td
index 704faf0ccd856..79c1a5cfd9aca 100644
--- a/flang/include/flang/Optimizer/OpenMP/Passes.td
+++ b/flang/include/flang/Optimizer/OpenMP/Passes.td
@@ -112,4 +112,9 @@ def GenericLoopConversionPass
];
}
+def SimdOnlyPass : Pass<"omp-simd-only", "mlir::func::FuncOp"> {
+ let summary = "Filters out non-simd OpenMP constructs";
+ let dependentDialects = ["mlir::omp::OpenMPDialect"];
+}
+
#endif //FORTRAN_OPTIMIZER_OPENMP_PASSES
diff --git a/flang/include/flang/Optimizer/Passes/Pipelines.h b/flang/include/flang/Optimizer/Passes/Pipelines.h
index a3f59ee8dd013..fd8c43cc88a19 100644
--- a/flang/include/flang/Optimizer/Passes/Pipelines.h
+++ b/flang/include/flang/Optimizer/Passes/Pipelines.h
@@ -119,13 +119,16 @@ void registerDefaultInlinerPass(MLIRToLLVMPassPipelineConfig &config);
void createDefaultFIROptimizerPassPipeline(mlir::PassManager &pm,
MLIRToLLVMPassPipelineConfig &pc);
+/// Select which mode to enable OpenMP support in.
+enum class EnableOpenMP { None, Simd, Full };
+
/// Create a pass pipeline for lowering from HLFIR to FIR
///
/// \param pm - MLIR pass manager that will hold the pipeline definition
/// \param optLevel - optimization level used for creating FIR optimization
/// passes pipeline
void createHLFIRToFIRPassPipeline(
- mlir::PassManager &pm, bool enableOpenMP,
+ mlir::PassManager &pm, EnableOpenMP enableOpenMP,
llvm::OptimizationLevel optLevel = defaultOptLevel);
struct OpenMPFIRPassPipelineOpts {
diff --git a/flang/include/flang/Optimizer/Transforms/Utils.h b/flang/include/flang/Optimizer/Transforms/Utils.h
index 49a616fb40fd5..307e6b59c57d4 100644
--- a/flang/include/flang/Optimizer/Transforms/Utils.h
+++ b/flang/include/flang/Optimizer/Transforms/Utils.h
@@ -33,6 +33,10 @@ void genMinMaxlocReductionLoop(fir::FirOpBuilder &builder, mlir::Value array,
mlir::Type maskElemType, mlir::Value resultArr,
bool maskMayBeLogicalScalar);
+std::pair<mlir::Block *, mlir::Block *>
+convertDoLoopToCFG(DoLoopOp loop, mlir::PatternRewriter &rewriter, bool setNSW,
+ bool forceLoopToExecuteOnce);
+
} // namespace fir
#endif // FORTRAN_OPTIMIZER_TRANSFORMS_UTILS_H
diff --git a/flang/include/flang/Support/LangOptions.def b/flang/include/flang/Support/LangOptions.def
index d5bf7a2ecc036..ba72d7b4b7212 100644
--- a/flang/include/flang/Support/LangOptions.def
+++ b/flang/include/flang/Support/LangOptions.def
@@ -58,6 +58,8 @@ LANGOPT(OpenMPTeamSubscription, 1, 0)
LANGOPT(OpenMPNoThreadState, 1, 0)
/// Assume that no thread in a parallel region will encounter a parallel region
LANGOPT(OpenMPNoNestedParallelism, 1, 0)
+/// Use SIMD only OpenMP support.
+LANGOPT(OpenMPSimd, 1, false)
LANGOPT(VScaleMin, 32, 0) ///< Minimum vscale range value
LANGOPT(VScaleMax, 32, 0) ///< Maximum vscale range value
diff --git a/flang/include/flang/Tools/CrossToolHelpers.h b/flang/include/flang/Tools/CrossToolHelpers.h
index df1da27058552..51958fa36c3ad 100644
--- a/flang/include/flang/Tools/CrossToolHelpers.h
+++ b/flang/include/flang/Tools/CrossToolHelpers.h
@@ -134,6 +134,7 @@ struct MLIRToLLVMPassPipelineConfig : public FlangEPCallBacks {
///< functions.
bool NSWOnLoopVarInc = true; ///< Add nsw flag to loop variable increments.
bool EnableOpenMP = false; ///< Enable OpenMP lowering.
+ bool EnableOpenMPSimd = false; ///< Enable OpenMP simd-only mode.
std::string InstrumentFunctionEntry =
""; ///< Name of the instrument-function that is called on each
///< function-entry
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index f55d866435997..80fd52b170f0c 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -1152,8 +1152,15 @@ static bool parseOpenMPArgs(CompilerInvocation &res, llvm::opt::ArgList &args,
clang::DiagnosticsEngine &diags) {
llvm::opt::Arg *arg = args.getLastArg(clang::driver::options::OPT_fopenmp,
clang::driver::options::OPT_fno_openmp);
- if (!arg || arg->getOption().matches(clang::driver::options::OPT_fno_openmp))
- return true;
+ if (!arg ||
+ arg->getOption().matches(clang::driver::options::OPT_fno_openmp)) {
+ bool isSimdSpecified = args.hasFlag(
+ clang::driver::options::OPT_fopenmp_simd,
+ clang::driver::options::OPT_fno_openmp_simd, /*Default=*/false);
+ if (!isSimdSpecified)
+ return true;
+ res.getLangOpts().OpenMPSimd = 1;
+ }
unsigned numErrorsBefore = diags.getNumErrors();
llvm::Triple t(res.getTargetOpts().triple);
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index b5f4f9421f633..0ac4c7094ec3b 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -297,6 +297,7 @@ bool CodeGenAction::beginSourceFileAction() {
bool isOpenMPEnabled =
ci.getInvocation().getFrontendOpts().features.IsEnabled(
Fortran::common::LanguageFeature::OpenMP);
+ bool isOpenMPSimd = ci.getInvocation().getLangOpts().OpenMPSimd;
fir::OpenMPFIRPassPipelineOpts opts;
@@ -328,12 +329,13 @@ bool CodeGenAction::beginSourceFileAction() {
if (auto offloadMod = llvm::dyn_cast<mlir::omp::OffloadModuleInterface>(
mlirModule->getOperation()))
opts.isTargetDevice = offloadMod.getIsTargetDevice();
+ }
- // WARNING: This pipeline must be run immediately after the lowering to
- // ensure that the FIR is correct with respect to OpenMP operations/
- // attributes.
+ // WARNING: This pipeline must be run immediately after the lowering to
+ // ensure that the FIR is correct with respect to OpenMP operations/
+ // attributes.
+ if (isOpenMPEnabled || isOpenMPSimd)
fir::createOpenMPFIRPassPipeline(pm, opts);
- }
pm.enableVerifier(/*verifyPasses=*/true);
pm.addPass(std::make_unique<Fortran::lower::VerifierPass>());
@@ -616,12 +618,14 @@ void CodeGenAction::lowerHLFIRToFIR() {
pm.addPass(std::make_unique<Fortran::lower::VerifierPass>());
pm.enableVerifier(/*verifyPasses=*/true);
+ fir::EnableOpenMP enableOpenMP = fir::EnableOpenMP::None;
+ if (ci.getInvocation().getFrontendOpts().features.IsEnabled(
+ Fortran::common::LanguageFeature::OpenMP))
+ enableOpenMP = fir::EnableOpenMP::Full;
+ if (ci.getInvocation().getLangOpts().OpenMPSimd)
+ enableOpenMP = fir::EnableOpenMP::Simd;
// Create the pass pipeline
- fir::createHLFIRToFIRPassPipeline(
- pm,
- ci.getInvocation().getFrontendOpts().features.IsEnabled(
- Fortran::common::LanguageFeature::OpenMP),
- level);
+ fir::createHLFIRToFIRPassPipeline(pm, enableOpenMP, level);
(void)mlir::applyPassManagerCLOptions(pm);
mlir::TimingScope timingScopeMLIRPasses = timingScopeRoot.nest(
@@ -747,6 +751,9 @@ void CodeGenAction::generateLLVMIR() {
Fortran::common::LanguageFeature::OpenMP))
config.EnableOpenMP = true;
+ if (ci.getInvocation().getLangOpts().OpenMPSimd)
+ config.EnableOpenMPSimd = true;
+
if (ci.getInvocation().getLoweringOpts().getIntegerWrapAround())
config.NSWOnLoopVarInc = false;
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.h b/flang/lib/Lower/OpenMP/ClauseProcessor.h
index f8a1f7983b79b..3e81f759ae69c 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.h
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.h
@@ -208,11 +208,12 @@ void ClauseProcessor::processTODO(mlir::Location currentLocation,
if (!x)
return;
unsigned version = semaCtx.langOptions().OpenMPVersion;
- TODO(currentLocation,
- "Unhandled clause " + llvm::omp::getOpenMPClauseName(id).upper() +
- " in " +
- llvm::omp::getOpenMPDirectiveName(directive, version).upper() +
- " construct");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(currentLocation,
+ "Unhandled clause " + llvm::omp::getOpenMPClauseName(id).upper() +
+ " in " +
+ llvm::omp::getOpenMPDirectiveName(directive, version).upper() +
+ " construct");
};
for (ClauseIterator it = clauses.begin(); it != clauses.end(); ++it)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 4c2d7badef382..1647b8f516e46 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -2259,7 +2259,8 @@ genOrderedOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
mlir::Location loc, const ConstructQueue &queue,
ConstructQueue::const_iterator item) {
- TODO(loc, "OMPD_ordered");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(loc, "OMPD_ordered");
return nullptr;
}
@@ -2446,7 +2447,8 @@ genScopeOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
mlir::Location loc, const ConstructQueue &queue,
ConstructQueue::const_iterator item) {
- TODO(loc, "Scope construct");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(loc, "Scope construct");
return nullptr;
}
@@ -3238,7 +3240,8 @@ static mlir::omp::TaskloopOp genCompositeTaskloopSimd(
lower::pft::Evaluation &eval, mlir::Location loc,
const ConstructQueue &queue, ConstructQueue::const_iterator item) {
assert(std::distance(item, queue.end()) == 2 && "Invalid leaf constructs");
- TODO(loc, "Composite TASKLOOP SIMD");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(loc, "Composite TASKLOOP SIMD");
return nullptr;
}
@@ -3410,8 +3413,10 @@ static void genOMPDispatch(lower::AbstractConverter &converter,
break;
case llvm::omp::Directive::OMPD_tile: {
unsigned version = semaCtx.langOptions().OpenMPVersion;
- TODO(loc, "Unhandled loop directive (" +
- llvm::omp::getOpenMPDirectiveName(dir, version) + ")");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(loc, "Unhandled loop directive (" +
+ llvm::omp::getOpenMPDirectiveName(dir, version) + ")");
+ break;
}
case llvm::omp::Directive::OMPD_unroll:
genUnrollOp(converter, symTable, stmtCtx, semaCtx, eval, loc, queue, item);
@@ -3446,35 +3451,40 @@ static void
genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
const parser::OpenMPDeclarativeAllocate &declarativeAllocate) {
- TODO(converter.getCurrentLocation(), "OpenMPDeclarativeAllocate");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(converter.getCurrentLocation(), "OpenMPDeclarativeAllocate");
}
static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval,
const parser::OpenMPDeclarativeAssumes &assumesConstruct) {
- TODO(converter.getCurrentLocation(), "OpenMP ASSUMES declaration");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(converter.getCurrentLocation(), "OpenMP ASSUMES declaration");
}
static void
genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
const parser::OmpDeclareVariantDirective &declareVariantDirective) {
- TODO(converter.getCurrentLocation(), "OmpDeclareVariantDirective");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(converter.getCurrentLocation(), "OmpDeclareVariantDirective");
}
static void genOMP(
lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
const parser::OpenMPDeclareReductionConstruct &declareReductionConstruct) {
- TODO(converter.getCurrentLocation(), "OpenMPDeclareReductionConstruct");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(converter.getCurrentLocation(), "OpenMPDeclareReductionConstruct");
}
static void
genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
const parser::OpenMPDeclareSimdConstruct &declareSimdConstruct) {
- TODO(converter.getCurrentLocation(), "OpenMPDeclareSimdConstruct");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(converter.getCurrentLocation(), "OpenMPDeclareSimdConstruct");
}
static void
@@ -3670,14 +3680,16 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
(void)objects;
(void)clauses;
- TODO(converter.getCurrentLocation(), "OpenMPDepobjConstruct");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(converter.getCurrentLocation(), "OpenMPDepobjConstruct");
}
static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval,
const parser::OpenMPInteropConstruct &interopConstruct) {
- TODO(converter.getCurrentLocation(), "OpenMPInteropConstruct");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(converter.getCurrentLocation(), "OpenMPInteropConstruct");
}
static void
@@ -3693,7 +3705,8 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval,
const parser::OpenMPAllocatorsConstruct &allocsConstruct) {
- TODO(converter.getCurrentLocation(), "OpenMPAllocatorsConstruct");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(converter.getCurrentLocation(), "OpenMPAllocatorsConstruct");
}
//===----------------------------------------------------------------------===//
@@ -3765,7 +3778,8 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
!std::holds_alternative<clause::Detach>(clause.u)) {
std::string name =
parser::ToUpperCaseLetters(llvm::omp::getOpenMPClauseName(clause.id));
- TODO(clauseLocation, name + " clause is not implemented yet");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(clauseLocation, name + " clause is not implemented yet");
}
}
@@ -3785,7 +3799,8 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
lower::pft::Evaluation &eval,
const parser::OpenMPAssumeConstruct &assumeConstruct) {
mlir::Location clauseLocation = converter.genLocation(assumeConstruct.source);
- TODO(clauseLocation, "OpenMP ASSUME construct");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(clauseLocation, "OpenMP ASSUME construct");
}
static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
@@ -3810,21 +3825,24 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval,
const parser::OpenMPUtilityConstruct &) {
- TODO(converter.getCurrentLocation(), "OpenMPUtilityConstruct");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(converter.getCurrentLocation(), "OpenMPUtilityConstruct");
}
static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval,
const parser::OpenMPDispatchConstruct &) {
- TODO(converter.getCurrentLocation(), "OpenMPDispatchConstruct");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(converter.getCurrentLocation(), "OpenMPDispatchConstruct");
}
static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval,
const parser::OpenMPExecutableAllocate &execAllocConstruct) {
- TODO(converter.getCurrentLocation(), "OpenMPExecutableAllocate");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(converter.getCurrentLocation(), "OpenMPExecutableAllocate");
}
static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
diff --git a/flang/lib/Optimizer/OpenMP/CMakeLists.txt b/flang/lib/Optimizer/OpenMP/CMakeLists.txt
index e31543328a9f9..3fb0bac05ce0d 100644
--- a/flang/lib/Optimizer/OpenMP/CMakeLists.txt
+++ b/flang/lib/Optimizer/OpenMP/CMakeLists.txt
@@ -9,6 +9,7 @@ add_flang_library(FlangOpenMPTransforms
MarkDeclareTarget.cpp
LowerWorkshare.cpp
LowerNontemporal.cpp
+ SimdOnly.cpp
DEPENDS
FIRDialect
diff --git a/flang/lib/Optimizer/OpenMP/SimdOnly.cpp b/flang/lib/Optimizer/OpenMP/SimdOnly.cpp
new file mode 100644
index 0000000000000..b4c97df767e65
--- /dev/null
+++ b/flang/lib/Optimizer/OpenMP/SimdOnly.cpp
@@ -0,0 +1,360 @@
+#include "flang/Optimizer/Builder/FIRBuilder.h"
+#include "flang/Optimizer/Transforms/Utils.h"
+#include "mlir/Dialect/ControlFlow/IR/ControlFlowOps.h"
+#include "mlir/Dialect/Func/IR/FuncOps.h"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
+#include "mlir/IR/IRMapping.h"
+#include "mlir/Pass/Pass.h"
+#include "mlir/Transforms/DialectConversion.h"
+#include "mlir/Transforms/GreedyPatternRewriteDriver.h"
+#include <llvm/Support/Debug.h>
+#include <mlir/IR/MLIRContext.h>
+#include <mlir/IR/Operation.h>
+#include <m...
[truncated]
|
@llvm/pr-subscribers-flang-openmp Author: Kajetan Puchalski (mrkajetanp) ChangesBoth clang and gfortran support the -fopenmp-simd flag, which enables OpenMP support only for simd constructs, while disabling the rest of OpenMP. Add a new SimdOnly flang OpenMP IR pass which rewrites generated OpenMP FIR to remove all constructs except for omp.simd constructs, and constructs nested under them. The flag is expected to have no effect if -fopenmp is passed explicitly, and is only expected to remove OpenMP constructs, not things like OpenMP library functions calls. This matches the behaviour of other compilers. Patch is 70.32 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/150269.diff 18 Files Affected:
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 916400efdb449..7a74dcffde4a9 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3706,14 +3706,19 @@ def fopenmp_relocatable_target : Flag<["-"], "fopenmp-relocatable-target">,
def fnoopenmp_relocatable_target : Flag<["-"], "fnoopenmp-relocatable-target">,
Group<f_Group>, Flags<[NoArgumentUnused, HelpHidden]>,
Visibility<[ClangOption, CC1Option]>;
-def fopenmp_simd : Flag<["-"], "fopenmp-simd">, Group<f_Group>,
- Flags<[NoArgumentUnused]>, Visibility<[ClangOption, CC1Option]>,
- HelpText<"Emit OpenMP code only for SIMD-based constructs.">;
+def fopenmp_simd : Flag<["-"], "fopenmp-simd">,
+ Group<f_Group>,
+ Flags<[NoArgumentUnused]>,
+ Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>,
+ HelpText<"Emit OpenMP code only for SIMD-based constructs.">;
def fopenmp_enable_irbuilder : Flag<["-"], "fopenmp-enable-irbuilder">, Group<f_Group>,
Flags<[NoArgumentUnused, HelpHidden]>, Visibility<[ClangOption, CC1Option]>,
HelpText<"Use the experimental OpenMP-IR-Builder codegen path.">;
-def fno_openmp_simd : Flag<["-"], "fno-openmp-simd">, Group<f_Group>,
- Flags<[NoArgumentUnused]>, Visibility<[ClangOption, CC1Option]>;
+def fno_openmp_simd
+ : Flag<["-"], "fno-openmp-simd">,
+ Group<f_Group>,
+ Flags<[NoArgumentUnused]>,
+ Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>;
def fopenmp_cuda_mode : Flag<["-"], "fopenmp-cuda-mode">, Group<f_Group>,
Flags<[NoArgumentUnused, HelpHidden]>, Visibility<[ClangOption, CC1Option]>;
def fno_openmp_cuda_mode : Flag<["-"], "fno-openmp-cuda-mode">, Group<f_Group>,
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index 7ab41e9b85a04..547e3156f519a 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -937,6 +937,8 @@ void Flang::ConstructJob(Compilation &C, const JobAction &JA,
if (Args.hasArg(options::OPT_fopenmp_force_usm))
CmdArgs.push_back("-fopenmp-force-usm");
+ Args.AddLastArg(CmdArgs, options::OPT_fopenmp_simd,
+ options::OPT_fno_openmp_simd);
// FIXME: Clang supports a whole bunch more flags here.
break;
@@ -952,6 +954,9 @@ void Flang::ConstructJob(Compilation &C, const JobAction &JA,
<< A->getSpelling() << A->getValue();
break;
}
+ } else {
+ Args.AddLastArg(CmdArgs, options::OPT_fopenmp_simd,
+ options::OPT_fno_openmp_simd);
}
// Pass the path to compiler resource files.
diff --git a/flang/include/flang/Optimizer/OpenMP/Passes.td b/flang/include/flang/Optimizer/OpenMP/Passes.td
index 704faf0ccd856..79c1a5cfd9aca 100644
--- a/flang/include/flang/Optimizer/OpenMP/Passes.td
+++ b/flang/include/flang/Optimizer/OpenMP/Passes.td
@@ -112,4 +112,9 @@ def GenericLoopConversionPass
];
}
+def SimdOnlyPass : Pass<"omp-simd-only", "mlir::func::FuncOp"> {
+ let summary = "Filters out non-simd OpenMP constructs";
+ let dependentDialects = ["mlir::omp::OpenMPDialect"];
+}
+
#endif //FORTRAN_OPTIMIZER_OPENMP_PASSES
diff --git a/flang/include/flang/Optimizer/Passes/Pipelines.h b/flang/include/flang/Optimizer/Passes/Pipelines.h
index a3f59ee8dd013..fd8c43cc88a19 100644
--- a/flang/include/flang/Optimizer/Passes/Pipelines.h
+++ b/flang/include/flang/Optimizer/Passes/Pipelines.h
@@ -119,13 +119,16 @@ void registerDefaultInlinerPass(MLIRToLLVMPassPipelineConfig &config);
void createDefaultFIROptimizerPassPipeline(mlir::PassManager &pm,
MLIRToLLVMPassPipelineConfig &pc);
+/// Select which mode to enable OpenMP support in.
+enum class EnableOpenMP { None, Simd, Full };
+
/// Create a pass pipeline for lowering from HLFIR to FIR
///
/// \param pm - MLIR pass manager that will hold the pipeline definition
/// \param optLevel - optimization level used for creating FIR optimization
/// passes pipeline
void createHLFIRToFIRPassPipeline(
- mlir::PassManager &pm, bool enableOpenMP,
+ mlir::PassManager &pm, EnableOpenMP enableOpenMP,
llvm::OptimizationLevel optLevel = defaultOptLevel);
struct OpenMPFIRPassPipelineOpts {
diff --git a/flang/include/flang/Optimizer/Transforms/Utils.h b/flang/include/flang/Optimizer/Transforms/Utils.h
index 49a616fb40fd5..307e6b59c57d4 100644
--- a/flang/include/flang/Optimizer/Transforms/Utils.h
+++ b/flang/include/flang/Optimizer/Transforms/Utils.h
@@ -33,6 +33,10 @@ void genMinMaxlocReductionLoop(fir::FirOpBuilder &builder, mlir::Value array,
mlir::Type maskElemType, mlir::Value resultArr,
bool maskMayBeLogicalScalar);
+std::pair<mlir::Block *, mlir::Block *>
+convertDoLoopToCFG(DoLoopOp loop, mlir::PatternRewriter &rewriter, bool setNSW,
+ bool forceLoopToExecuteOnce);
+
} // namespace fir
#endif // FORTRAN_OPTIMIZER_TRANSFORMS_UTILS_H
diff --git a/flang/include/flang/Support/LangOptions.def b/flang/include/flang/Support/LangOptions.def
index d5bf7a2ecc036..ba72d7b4b7212 100644
--- a/flang/include/flang/Support/LangOptions.def
+++ b/flang/include/flang/Support/LangOptions.def
@@ -58,6 +58,8 @@ LANGOPT(OpenMPTeamSubscription, 1, 0)
LANGOPT(OpenMPNoThreadState, 1, 0)
/// Assume that no thread in a parallel region will encounter a parallel region
LANGOPT(OpenMPNoNestedParallelism, 1, 0)
+/// Use SIMD only OpenMP support.
+LANGOPT(OpenMPSimd, 1, false)
LANGOPT(VScaleMin, 32, 0) ///< Minimum vscale range value
LANGOPT(VScaleMax, 32, 0) ///< Maximum vscale range value
diff --git a/flang/include/flang/Tools/CrossToolHelpers.h b/flang/include/flang/Tools/CrossToolHelpers.h
index df1da27058552..51958fa36c3ad 100644
--- a/flang/include/flang/Tools/CrossToolHelpers.h
+++ b/flang/include/flang/Tools/CrossToolHelpers.h
@@ -134,6 +134,7 @@ struct MLIRToLLVMPassPipelineConfig : public FlangEPCallBacks {
///< functions.
bool NSWOnLoopVarInc = true; ///< Add nsw flag to loop variable increments.
bool EnableOpenMP = false; ///< Enable OpenMP lowering.
+ bool EnableOpenMPSimd = false; ///< Enable OpenMP simd-only mode.
std::string InstrumentFunctionEntry =
""; ///< Name of the instrument-function that is called on each
///< function-entry
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index f55d866435997..80fd52b170f0c 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -1152,8 +1152,15 @@ static bool parseOpenMPArgs(CompilerInvocation &res, llvm::opt::ArgList &args,
clang::DiagnosticsEngine &diags) {
llvm::opt::Arg *arg = args.getLastArg(clang::driver::options::OPT_fopenmp,
clang::driver::options::OPT_fno_openmp);
- if (!arg || arg->getOption().matches(clang::driver::options::OPT_fno_openmp))
- return true;
+ if (!arg ||
+ arg->getOption().matches(clang::driver::options::OPT_fno_openmp)) {
+ bool isSimdSpecified = args.hasFlag(
+ clang::driver::options::OPT_fopenmp_simd,
+ clang::driver::options::OPT_fno_openmp_simd, /*Default=*/false);
+ if (!isSimdSpecified)
+ return true;
+ res.getLangOpts().OpenMPSimd = 1;
+ }
unsigned numErrorsBefore = diags.getNumErrors();
llvm::Triple t(res.getTargetOpts().triple);
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index b5f4f9421f633..0ac4c7094ec3b 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -297,6 +297,7 @@ bool CodeGenAction::beginSourceFileAction() {
bool isOpenMPEnabled =
ci.getInvocation().getFrontendOpts().features.IsEnabled(
Fortran::common::LanguageFeature::OpenMP);
+ bool isOpenMPSimd = ci.getInvocation().getLangOpts().OpenMPSimd;
fir::OpenMPFIRPassPipelineOpts opts;
@@ -328,12 +329,13 @@ bool CodeGenAction::beginSourceFileAction() {
if (auto offloadMod = llvm::dyn_cast<mlir::omp::OffloadModuleInterface>(
mlirModule->getOperation()))
opts.isTargetDevice = offloadMod.getIsTargetDevice();
+ }
- // WARNING: This pipeline must be run immediately after the lowering to
- // ensure that the FIR is correct with respect to OpenMP operations/
- // attributes.
+ // WARNING: This pipeline must be run immediately after the lowering to
+ // ensure that the FIR is correct with respect to OpenMP operations/
+ // attributes.
+ if (isOpenMPEnabled || isOpenMPSimd)
fir::createOpenMPFIRPassPipeline(pm, opts);
- }
pm.enableVerifier(/*verifyPasses=*/true);
pm.addPass(std::make_unique<Fortran::lower::VerifierPass>());
@@ -616,12 +618,14 @@ void CodeGenAction::lowerHLFIRToFIR() {
pm.addPass(std::make_unique<Fortran::lower::VerifierPass>());
pm.enableVerifier(/*verifyPasses=*/true);
+ fir::EnableOpenMP enableOpenMP = fir::EnableOpenMP::None;
+ if (ci.getInvocation().getFrontendOpts().features.IsEnabled(
+ Fortran::common::LanguageFeature::OpenMP))
+ enableOpenMP = fir::EnableOpenMP::Full;
+ if (ci.getInvocation().getLangOpts().OpenMPSimd)
+ enableOpenMP = fir::EnableOpenMP::Simd;
// Create the pass pipeline
- fir::createHLFIRToFIRPassPipeline(
- pm,
- ci.getInvocation().getFrontendOpts().features.IsEnabled(
- Fortran::common::LanguageFeature::OpenMP),
- level);
+ fir::createHLFIRToFIRPassPipeline(pm, enableOpenMP, level);
(void)mlir::applyPassManagerCLOptions(pm);
mlir::TimingScope timingScopeMLIRPasses = timingScopeRoot.nest(
@@ -747,6 +751,9 @@ void CodeGenAction::generateLLVMIR() {
Fortran::common::LanguageFeature::OpenMP))
config.EnableOpenMP = true;
+ if (ci.getInvocation().getLangOpts().OpenMPSimd)
+ config.EnableOpenMPSimd = true;
+
if (ci.getInvocation().getLoweringOpts().getIntegerWrapAround())
config.NSWOnLoopVarInc = false;
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.h b/flang/lib/Lower/OpenMP/ClauseProcessor.h
index f8a1f7983b79b..3e81f759ae69c 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.h
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.h
@@ -208,11 +208,12 @@ void ClauseProcessor::processTODO(mlir::Location currentLocation,
if (!x)
return;
unsigned version = semaCtx.langOptions().OpenMPVersion;
- TODO(currentLocation,
- "Unhandled clause " + llvm::omp::getOpenMPClauseName(id).upper() +
- " in " +
- llvm::omp::getOpenMPDirectiveName(directive, version).upper() +
- " construct");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(currentLocation,
+ "Unhandled clause " + llvm::omp::getOpenMPClauseName(id).upper() +
+ " in " +
+ llvm::omp::getOpenMPDirectiveName(directive, version).upper() +
+ " construct");
};
for (ClauseIterator it = clauses.begin(); it != clauses.end(); ++it)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 4c2d7badef382..1647b8f516e46 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -2259,7 +2259,8 @@ genOrderedOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
mlir::Location loc, const ConstructQueue &queue,
ConstructQueue::const_iterator item) {
- TODO(loc, "OMPD_ordered");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(loc, "OMPD_ordered");
return nullptr;
}
@@ -2446,7 +2447,8 @@ genScopeOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
mlir::Location loc, const ConstructQueue &queue,
ConstructQueue::const_iterator item) {
- TODO(loc, "Scope construct");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(loc, "Scope construct");
return nullptr;
}
@@ -3238,7 +3240,8 @@ static mlir::omp::TaskloopOp genCompositeTaskloopSimd(
lower::pft::Evaluation &eval, mlir::Location loc,
const ConstructQueue &queue, ConstructQueue::const_iterator item) {
assert(std::distance(item, queue.end()) == 2 && "Invalid leaf constructs");
- TODO(loc, "Composite TASKLOOP SIMD");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(loc, "Composite TASKLOOP SIMD");
return nullptr;
}
@@ -3410,8 +3413,10 @@ static void genOMPDispatch(lower::AbstractConverter &converter,
break;
case llvm::omp::Directive::OMPD_tile: {
unsigned version = semaCtx.langOptions().OpenMPVersion;
- TODO(loc, "Unhandled loop directive (" +
- llvm::omp::getOpenMPDirectiveName(dir, version) + ")");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(loc, "Unhandled loop directive (" +
+ llvm::omp::getOpenMPDirectiveName(dir, version) + ")");
+ break;
}
case llvm::omp::Directive::OMPD_unroll:
genUnrollOp(converter, symTable, stmtCtx, semaCtx, eval, loc, queue, item);
@@ -3446,35 +3451,40 @@ static void
genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
const parser::OpenMPDeclarativeAllocate &declarativeAllocate) {
- TODO(converter.getCurrentLocation(), "OpenMPDeclarativeAllocate");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(converter.getCurrentLocation(), "OpenMPDeclarativeAllocate");
}
static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval,
const parser::OpenMPDeclarativeAssumes &assumesConstruct) {
- TODO(converter.getCurrentLocation(), "OpenMP ASSUMES declaration");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(converter.getCurrentLocation(), "OpenMP ASSUMES declaration");
}
static void
genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
const parser::OmpDeclareVariantDirective &declareVariantDirective) {
- TODO(converter.getCurrentLocation(), "OmpDeclareVariantDirective");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(converter.getCurrentLocation(), "OmpDeclareVariantDirective");
}
static void genOMP(
lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
const parser::OpenMPDeclareReductionConstruct &declareReductionConstruct) {
- TODO(converter.getCurrentLocation(), "OpenMPDeclareReductionConstruct");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(converter.getCurrentLocation(), "OpenMPDeclareReductionConstruct");
}
static void
genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
const parser::OpenMPDeclareSimdConstruct &declareSimdConstruct) {
- TODO(converter.getCurrentLocation(), "OpenMPDeclareSimdConstruct");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(converter.getCurrentLocation(), "OpenMPDeclareSimdConstruct");
}
static void
@@ -3670,14 +3680,16 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
(void)objects;
(void)clauses;
- TODO(converter.getCurrentLocation(), "OpenMPDepobjConstruct");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(converter.getCurrentLocation(), "OpenMPDepobjConstruct");
}
static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval,
const parser::OpenMPInteropConstruct &interopConstruct) {
- TODO(converter.getCurrentLocation(), "OpenMPInteropConstruct");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(converter.getCurrentLocation(), "OpenMPInteropConstruct");
}
static void
@@ -3693,7 +3705,8 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval,
const parser::OpenMPAllocatorsConstruct &allocsConstruct) {
- TODO(converter.getCurrentLocation(), "OpenMPAllocatorsConstruct");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(converter.getCurrentLocation(), "OpenMPAllocatorsConstruct");
}
//===----------------------------------------------------------------------===//
@@ -3765,7 +3778,8 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
!std::holds_alternative<clause::Detach>(clause.u)) {
std::string name =
parser::ToUpperCaseLetters(llvm::omp::getOpenMPClauseName(clause.id));
- TODO(clauseLocation, name + " clause is not implemented yet");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(clauseLocation, name + " clause is not implemented yet");
}
}
@@ -3785,7 +3799,8 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
lower::pft::Evaluation &eval,
const parser::OpenMPAssumeConstruct &assumeConstruct) {
mlir::Location clauseLocation = converter.genLocation(assumeConstruct.source);
- TODO(clauseLocation, "OpenMP ASSUME construct");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(clauseLocation, "OpenMP ASSUME construct");
}
static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
@@ -3810,21 +3825,24 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval,
const parser::OpenMPUtilityConstruct &) {
- TODO(converter.getCurrentLocation(), "OpenMPUtilityConstruct");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(converter.getCurrentLocation(), "OpenMPUtilityConstruct");
}
static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval,
const parser::OpenMPDispatchConstruct &) {
- TODO(converter.getCurrentLocation(), "OpenMPDispatchConstruct");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(converter.getCurrentLocation(), "OpenMPDispatchConstruct");
}
static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval,
const parser::OpenMPExecutableAllocate &execAllocConstruct) {
- TODO(converter.getCurrentLocation(), "OpenMPExecutableAllocate");
+ if (!semaCtx.langOptions().OpenMPSimd)
+ TODO(converter.getCurrentLocation(), "OpenMPExecutableAllocate");
}
static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
diff --git a/flang/lib/Optimizer/OpenMP/CMakeLists.txt b/flang/lib/Optimizer/OpenMP/CMakeLists.txt
index e31543328a9f9..3fb0bac05ce0d 100644
--- a/flang/lib/Optimizer/OpenMP/CMakeLists.txt
+++ b/flang/lib/Optimizer/OpenMP/CMakeLists.txt
@@ -9,6 +9,7 @@ add_flang_library(FlangOpenMPTransforms
MarkDeclareTarget.cpp
LowerWorkshare.cpp
LowerNontemporal.cpp
+ SimdOnly.cpp
DEPENDS
FIRDialect
diff --git a/flang/lib/Optimizer/OpenMP/SimdOnly.cpp b/flang/lib/Optimizer/OpenMP/SimdOnly.cpp
new file mode 100644
index 0000000000000..b4c97df767e65
--- /dev/null
+++ b/flang/lib/Optimizer/OpenMP/SimdOnly.cpp
@@ -0,0 +1,360 @@
+#include "flang/Optimizer/Builder/FIRBuilder.h"
+#include "flang/Optimizer/Transforms/Utils.h"
+#include "mlir/Dialect/ControlFlow/IR/ControlFlowOps.h"
+#include "mlir/Dialect/Func/IR/FuncOps.h"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
+#include "mlir/IR/IRMapping.h"
+#include "mlir/Pass/Pass.h"
+#include "mlir/Transforms/DialectConversion.h"
+#include "mlir/Transforms/GreedyPatternRewriteDriver.h"
+#include <llvm/Support/Debug.h>
+#include <mlir/IR/MLIRContext.h>
+#include <mlir/IR/Operation.h>
+#include <m...
[truncated]
|
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.
Thanks. The frontend parts of it look fine. I haven't looked too closely at the core OpenMP changes.
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 is great so far
Have you considered handling this in the prescanning/parsing, semantics or lowering stages? |
Thanks for the patch.
Is there a reason why we are preferring to generate constructs and then removing non-simd ones, as opposed to simply generating only simd? |
We could add checks to ignore semantic errors if the flag is set in the same way lowering TODOs are currently ignored if that's the behaviour we want. Still emitting semantic errors for non-simd OpenMP constructs is the current behaviour in clang:
|
I think both approaches have benefits and drawbacks, as usual. The current one keeps all the logic closer together as opposed to having a couple lines for handling this flag in the code for lowering every individual op, it makes it easier to maintain in some ways (the pass will emit an error if it sees any unhandled OpenMP ops, whereas otherwise an unhandled op would get silently left in and there would be nothing to check for it). It also guarantees that the simd construct and its associated ops have been generated correctly, whereas if we were selectively disabling the lowering for some ops there might be some more edge cases there to handle. It's a pretty niche feature in the grand scheme of things, hence I thought it would be better to keep the code for it in its own file rather than all over the core lowering logic. Wasted CPU cycles are the thing being traded off here, but given that it is a niche feature (probably mainly used for testing and experiments of some kinds) then I don't think it's prohibitive? |
✅ With the latest revision this PR passed the C/C++ code formatter. |
d77230f
to
f8f39f9
Compare
Thanks. Makes sense to me. |
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.
The driver side of things looks good to me. Please wait for others to approve the rest. Thanks.
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.
The overall approach LGTM so long as @kiranchandramohan is happy.
Have you looked at the new canonical loop operation omp.new_cli
(I think it was merged since you started this patch)? It isn't obvious if we should allow tile+unroll (and the OpenMP 6.0 additions) along with simd. I would guess yes but I think matching clang is most important.
8caae75
to
278bdb8
Compare
Pretty substantial redesign, I moved most of the logic into parse tree rewriting, I think this simplifies it and makes it more maintainable for the future. The rationale for the now-split design is as follows: Most OpenMP constructs do not relate to simd at all, and so they can just be removed straight away. It is much cleaner to remove them in the parsing stage, because we still have all of the information on what the original code was. For instance, OpenMP loop constructs have a plain DoConstruct nested underneath them, so we can just use it directly. Once it gets lowered into an OpenMP loop, we would need to reverse the lowering which is a lot more brittle to maintain and keep in sync with the main lowering path. With the parse tree approach, the generated FIR will always be identical to On the other hand, for composite simd constructs, the situation is the opposite. Constructs like "target teams distribute simd" are all one construct on the parse tree level, but are lowered into nested operations in MLIR. Trying to rewrite them into plain simd on the parse tree level would be brittle as well, and require a lot of special casing to make sure that we don't accidentally create a construct with illegal clauses that would then fail the semantic checks. With the MLIR pass, handling those nested operations is very straightforward. Keeping the MLIR pass also has the added benefit of having an extra check at the end that will complain to the user if any unexpected OpenMP ops are still left over at the end. I think this type of split design is the best way to make the flag maintainable, while trying to forcibly fit all the logic into either the MLIR or the parse tree would be more likely to cause problems down the line. |
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.
Looks great to me.
Just to elaborate on the need to keep the MLIR pass: the logic for which clauses apply to which directive in a compound construct is non-trivial. We thought it is better not to try to re-implement that here.
Both clang and gfortran support the -fopenmp-simd flag, which enables OpenMP support only for simd constructs, while disabling the rest of OpenMP. Add a new SimdOnly flang OpenMP IR pass which rewrites generated OpenMP FIR to remove all constructs except for omp.simd constructs, and constructs nested under them. With this approach, the logic required to make the flag work can be self-contained within the pass, as opposed to being scattered all over the lowering code. The flag is expected to have no effect if -fopenmp is passed explicitly, and is only expected to remove OpenMP constructs, not things like OpenMP library functions calls. This matches the behaviour of other compilers. Signed-off-by: Kajetan Puchalski <kajetan.puchalski@arm.com>
12da7e6
to
33d63a3
Compare
33d63a3
to
de02e17
Compare
Thank you! A small NIT. Otherwise LGTM. |
Both clang and gfortran support the -fopenmp-simd flag, which enables OpenMP support only for simd constructs, while disabling the rest of OpenMP.
Implement the appropriate parse tree rewriting to remove non-SIMD OpenMP constructs at the parsing stage.
Add a new SimdOnly flang OpenMP IR pass which rewrites generated OpenMP FIR to handle untangling composite simd constructs, and clean up OpenMP operations leftover after the parse tree rewriting stage.
With this approach, the two parts of the logic required to make the flag work can be self-contained within the parse tree rewriter and the MLIR pass, respectively. It does not need to be implemented within the core lowering logic itself.
The flag is expected to have no effect if -fopenmp is passed explicitly, and is only expected to remove OpenMP constructs, not things like OpenMP library functions calls. This matches the behaviour of other compilers.