[flang-commits] [flang] 7c385c4 - [mlir][OpenMP] Generating enums in accordance with the guidelines
Shraiysh Vaishay via flang-commits
flang-commits at lists.llvm.org
Wed Mar 9 06:40:57 PST 2022
Author: Shraiysh Vaishay
Date: 2022-03-09T20:10:45+05:30
New Revision: 7c385c4b2f653d6853fe4601f02bbc7fcd60cd91
URL: https://github.com/llvm/llvm-project/commit/7c385c4b2f653d6853fe4601f02bbc7fcd60cd91
DIFF: https://github.com/llvm/llvm-project/commit/7c385c4b2f653d6853fe4601f02bbc7fcd60cd91.diff
LOG: [mlir][OpenMP] Generating enums in accordance with the guidelines
This patch changes the enums generated from `OMP.td` for MLIR according
to the enum naming guidelines in LLVM Coding Standards.
This also helps the issues we had with `static` being a C++ keyword and
also a value for the schedule clause.
Enumerator naming guidelines: https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
Reviewed By: kiranchandramohan
Differential Revision: https://reviews.llvm.org/D120825
Added:
Modified:
flang/lib/Lower/OpenMP.cpp
llvm/include/llvm/Frontend/OpenMP/OMP.td
llvm/unittests/Frontend/OpenMPParsingTest.cpp
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
mlir/test/Dialect/OpenMP/ops.mlir
mlir/test/mlir-tblgen/directive-common.td
mlir/tools/mlir-tblgen/DirectiveCommonGen.cpp
Removed:
################################################################################
diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index 2c2ca3b529870..83e1a99f95ffd 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -181,13 +181,13 @@ genOMP(Fortran::lower::AbstractConverter &converter,
omp::ClauseProcBindKind pbKind;
switch (ompProcBindClause.v) {
case Fortran::parser::OmpProcBindClause::Type::Master:
- pbKind = omp::ClauseProcBindKind::master;
+ pbKind = omp::ClauseProcBindKind::Master;
break;
case Fortran::parser::OmpProcBindClause::Type::Close:
- pbKind = omp::ClauseProcBindKind::close;
+ pbKind = omp::ClauseProcBindKind::Close;
break;
case Fortran::parser::OmpProcBindClause::Type::Spread:
- pbKind = omp::ClauseProcBindKind::spread;
+ pbKind = omp::ClauseProcBindKind::Spread;
break;
}
parallelOp.proc_bind_valAttr(omp::ClauseProcBindKindAttr::get(
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td
index bdf4a9fd1dc75..3ff2a14b94781 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -122,13 +122,12 @@ def OMPC_ProcBind : Clause<"proc_bind"> {
];
}
-// static and auto are C++ keywords so need a capital to disambiguate.
-def OMP_SCHEDULE_Static : ClauseVal<"Static", 2, 1> {}
-def OMP_SCHEDULE_Dynamic : ClauseVal<"Dynamic", 3, 1> {}
-def OMP_SCHEDULE_Guided : ClauseVal<"Guided", 4, 1> {}
-def OMP_SCHEDULE_Auto : ClauseVal<"Auto", 5, 1> {}
-def OMP_SCHEDULE_Runtime : ClauseVal<"Runtime", 6, 1> {}
-def OMP_SCHEDULE_Default : ClauseVal<"Default", 7, 0> { let isDefault = 1; }
+def OMP_SCHEDULE_Static : ClauseVal<"static", 2, 1> {}
+def OMP_SCHEDULE_Dynamic : ClauseVal<"dynamic", 3, 1> {}
+def OMP_SCHEDULE_Guided : ClauseVal<"guided", 4, 1> {}
+def OMP_SCHEDULE_Auto : ClauseVal<"auto", 5, 1> {}
+def OMP_SCHEDULE_Runtime : ClauseVal<"runtime", 6, 1> {}
+def OMP_SCHEDULE_Default : ClauseVal<"default", 7, 0> { let isDefault = 1; }
def OMPC_Schedule : Clause<"schedule"> {
let clangClass = "OMPScheduleClause";
diff --git a/llvm/unittests/Frontend/OpenMPParsingTest.cpp b/llvm/unittests/Frontend/OpenMPParsingTest.cpp
index 227e08c511d13..80ec2f0ad4d32 100644
--- a/llvm/unittests/Frontend/OpenMPParsingTest.cpp
+++ b/llvm/unittests/Frontend/OpenMPParsingTest.cpp
@@ -73,13 +73,12 @@ TEST(OpenMPParsingTest, getProcBindKind) {
TEST(OpenMPParsingTest, getScheduleKind) {
EXPECT_EQ(getScheduleKind("foobar"), OMP_SCHEDULE_Default);
- // FIXME: Why are these not lower case?
- EXPECT_EQ(getScheduleKind("Static"), OMP_SCHEDULE_Static);
- EXPECT_EQ(getScheduleKind("Dynamic"), OMP_SCHEDULE_Dynamic);
- EXPECT_EQ(getScheduleKind("Guided"), OMP_SCHEDULE_Guided);
- EXPECT_EQ(getScheduleKind("Auto"), OMP_SCHEDULE_Auto);
- EXPECT_EQ(getScheduleKind("Runtime"), OMP_SCHEDULE_Runtime);
- EXPECT_EQ(getScheduleKind("Default"), OMP_SCHEDULE_Default);
+ EXPECT_EQ(getScheduleKind("static"), OMP_SCHEDULE_Static);
+ EXPECT_EQ(getScheduleKind("dynamic"), OMP_SCHEDULE_Dynamic);
+ EXPECT_EQ(getScheduleKind("guided"), OMP_SCHEDULE_Guided);
+ EXPECT_EQ(getScheduleKind("auto"), OMP_SCHEDULE_Auto);
+ EXPECT_EQ(getScheduleKind("runtime"), OMP_SCHEDULE_Runtime);
+ EXPECT_EQ(getScheduleKind("default"), OMP_SCHEDULE_Default);
}
} // namespace
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index b4e08ac7b9a1c..87098b9f7421f 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -674,7 +674,6 @@ static ParseResult parseClauses(OpAsmParser &parser, OperationState &result,
// Add schedule parameters
if (done[scheduleClause] && !schedule.empty()) {
- schedule[0] = llvm::toUpper(schedule[0]);
if (Optional<ClauseScheduleKind> sched =
symbolizeClauseScheduleKind(schedule)) {
auto attr = ClauseScheduleKindAttr::get(parser.getContext(), *sched);
@@ -999,8 +998,8 @@ LogicalResult OrderedRegionOp::verify() {
LogicalResult AtomicReadOp::verify() {
if (auto mo = memory_order_val()) {
- if (*mo == ClauseMemoryOrderKind::acq_rel ||
- *mo == ClauseMemoryOrderKind::release) {
+ if (*mo == ClauseMemoryOrderKind::Acq_rel ||
+ *mo == ClauseMemoryOrderKind::Release) {
return emitError(
"memory-order must not be acq_rel or release for atomic reads");
}
@@ -1017,8 +1016,8 @@ LogicalResult AtomicReadOp::verify() {
LogicalResult AtomicWriteOp::verify() {
if (auto mo = memory_order_val()) {
- if (*mo == ClauseMemoryOrderKind::acq_rel ||
- *mo == ClauseMemoryOrderKind::acquire) {
+ if (*mo == ClauseMemoryOrderKind::Acq_rel ||
+ *mo == ClauseMemoryOrderKind::Acquire) {
return emitError(
"memory-order must not be acq_rel or acquire for atomic writes");
}
@@ -1032,8 +1031,8 @@ LogicalResult AtomicWriteOp::verify() {
LogicalResult AtomicUpdateOp::verify() {
if (auto mo = memory_order_val()) {
- if (*mo == ClauseMemoryOrderKind::acq_rel ||
- *mo == ClauseMemoryOrderKind::acquire) {
+ if (*mo == ClauseMemoryOrderKind::Acq_rel ||
+ *mo == ClauseMemoryOrderKind::Acquire) {
return emitError(
"memory-order must not be acq_rel or acquire for atomic updates");
}
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 5c19b995b6b6a..feaa75c0bc214 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -190,13 +190,13 @@ static void convertOmpOpRegions(
/// Convert ProcBindKind from MLIR-generated enum to LLVM enum.
static llvm::omp::ProcBindKind getProcBindKind(omp::ClauseProcBindKind kind) {
switch (kind) {
- case omp::ClauseProcBindKind::close:
+ case omp::ClauseProcBindKind::Close:
return llvm::omp::ProcBindKind::OMP_PROC_BIND_close;
- case omp::ClauseProcBindKind::master:
+ case omp::ClauseProcBindKind::Master:
return llvm::omp::ProcBindKind::OMP_PROC_BIND_master;
- case omp::ClauseProcBindKind::primary:
+ case omp::ClauseProcBindKind::Primary:
return llvm::omp::ProcBindKind::OMP_PROC_BIND_primary;
- case omp::ClauseProcBindKind::spread:
+ case omp::ClauseProcBindKind::Spread:
return llvm::omp::ProcBindKind::OMP_PROC_BIND_spread;
}
llvm_unreachable("Unknown ClauseProcBindKind kind");
@@ -887,15 +887,15 @@ convertAtomicOrdering(Optional<omp::ClauseMemoryOrderKind> ao) {
return llvm::AtomicOrdering::Monotonic; // Default Memory Ordering
switch (*ao) {
- case omp::ClauseMemoryOrderKind::seq_cst:
+ case omp::ClauseMemoryOrderKind::Seq_cst:
return llvm::AtomicOrdering::SequentiallyConsistent;
- case omp::ClauseMemoryOrderKind::acq_rel:
+ case omp::ClauseMemoryOrderKind::Acq_rel:
return llvm::AtomicOrdering::AcquireRelease;
- case omp::ClauseMemoryOrderKind::acquire:
+ case omp::ClauseMemoryOrderKind::Acquire:
return llvm::AtomicOrdering::Acquire;
- case omp::ClauseMemoryOrderKind::release:
+ case omp::ClauseMemoryOrderKind::Release:
return llvm::AtomicOrdering::Release;
- case omp::ClauseMemoryOrderKind::relaxed:
+ case omp::ClauseMemoryOrderKind::Relaxed:
return llvm::AtomicOrdering::Monotonic;
}
llvm_unreachable("Unknown ClauseMemoryOrderKind kind");
diff --git a/mlir/test/Dialect/OpenMP/ops.mlir b/mlir/test/Dialect/OpenMP/ops.mlir
index 3d6834f0d9345..62776e2db9d93 100644
--- a/mlir/test/Dialect/OpenMP/ops.mlir
+++ b/mlir/test/Dialect/OpenMP/ops.mlir
@@ -135,28 +135,28 @@ func @omp_wsloop(%lb : index, %ub : index, %step : index, %data_var : memref<i32
"omp.wsloop" (%lb, %ub, %step, %data_var, %linear_var) ({
^bb0(%iv: index):
omp.yield
- }) {operand_segment_sizes = dense<[1,1,1,1,1,0,0]> : vector<7xi32>, schedule_val = #omp<"schedulekind Static">} :
+ }) {operand_segment_sizes = dense<[1,1,1,1,1,0,0]> : vector<7xi32>, schedule_val = #omp<"schedulekind static">} :
(index, index, index, memref<i32>, i32) -> ()
// CHECK: omp.wsloop (%{{.*}}) : index = (%{{.*}}) to (%{{.*}}) step (%{{.*}}) linear(%{{.*}} = %{{.*}} : memref<i32>, %{{.*}} = %{{.*}} : memref<i32>) schedule(static)
"omp.wsloop" (%lb, %ub, %step, %data_var, %data_var, %linear_var, %linear_var) ({
^bb0(%iv: index):
omp.yield
- }) {operand_segment_sizes = dense<[1,1,1,2,2,0,0]> : vector<7xi32>, schedule_val = #omp<"schedulekind Static">} :
+ }) {operand_segment_sizes = dense<[1,1,1,2,2,0,0]> : vector<7xi32>, schedule_val = #omp<"schedulekind static">} :
(index, index, index, memref<i32>, memref<i32>, i32, i32) -> ()
// CHECK: omp.wsloop (%{{.*}}) : index = (%{{.*}}) to (%{{.*}}) step (%{{.*}}) linear(%{{.*}} = %{{.*}} : memref<i32>) schedule(dynamic = %{{.*}}) collapse(3) ordered(2)
"omp.wsloop" (%lb, %ub, %step, %data_var, %linear_var, %chunk_var) ({
^bb0(%iv: index):
omp.yield
- }) {operand_segment_sizes = dense<[1,1,1,1,1,0,1]> : vector<7xi32>, schedule_val = #omp<"schedulekind Dynamic">, collapse_val = 3, ordered_val = 2} :
+ }) {operand_segment_sizes = dense<[1,1,1,1,1,0,1]> : vector<7xi32>, schedule_val = #omp<"schedulekind dynamic">, collapse_val = 3, ordered_val = 2} :
(index, index, index, memref<i32>, i32, i32) -> ()
// CHECK: omp.wsloop (%{{.*}}) : index = (%{{.*}}) to (%{{.*}}) step (%{{.*}}) schedule(auto) nowait
"omp.wsloop" (%lb, %ub, %step) ({
^bb0(%iv: index):
omp.yield
- }) {operand_segment_sizes = dense<[1,1,1,0,0,0,0]> : vector<7xi32>, nowait, schedule_val = #omp<"schedulekind Auto">} :
+ }) {operand_segment_sizes = dense<[1,1,1,0,0,0,0]> : vector<7xi32>, nowait, schedule_val = #omp<"schedulekind auto">} :
(index, index, index) -> ()
return
diff --git a/mlir/test/mlir-tblgen/directive-common.td b/mlir/test/mlir-tblgen/directive-common.td
index b1d554c4c7038..dd86dea36417c 100644
--- a/mlir/test/mlir-tblgen/directive-common.td
+++ b/mlir/test/mlir-tblgen/directive-common.td
@@ -21,8 +21,8 @@ def TDLC_ClauseA : Clause<"clausea"> {
];
}
-// CHECK: def AKindvala : I32EnumAttrCase<"vala", 0>;
-// CHECK: def AKindvalb : I32EnumAttrCase<"valb", 1>;
+// CHECK: def AKindvala : I32EnumAttrCase<"Vala", 0, "vala">;
+// CHECK: def AKindvalb : I32EnumAttrCase<"Valb", 1, "valb">;
// CHECK: def AKind: I32EnumAttr<
// CHECK: "ClauseAKind",
// CHECK: "AKind Clause",
diff --git a/mlir/tools/mlir-tblgen/DirectiveCommonGen.cpp b/mlir/tools/mlir-tblgen/DirectiveCommonGen.cpp
index 4873f914afe49..a5c89344b2d61 100644
--- a/mlir/tools/mlir-tblgen/DirectiveCommonGen.cpp
+++ b/mlir/tools/mlir-tblgen/DirectiveCommonGen.cpp
@@ -70,10 +70,14 @@ static bool emitDecls(const RecordKeeper &recordKeeper, llvm::StringRef dialect,
if (!cval.isUserVisible())
continue;
- const auto name = cval.getFormattedName();
+ std::string name = cval.getFormattedName();
+ std::string enumValName(name.length(), ' ');
+ std::transform(name.begin(), name.end(), enumValName.begin(),
+ llvm::toLower);
+ enumValName[0] = llvm::toUpper(enumValName[0]);
std::string cvDef{(enumName + llvm::Twine(name)).str()};
- os << "def " << cvDef << " : I32EnumAttrCase<\"" << name << "\", "
- << it.index() << ">;\n";
+ os << "def " << cvDef << " : I32EnumAttrCase<\"" << enumValName << "\", "
+ << it.index() << ", \"" << name << "\">;\n";
cvDefs.push_back(cvDef);
}
More information about the flang-commits
mailing list