Skip to content

Commit 26d811b

Browse files
committed
[mlir] Make use of C++17 language features
Now that C++17 is enabled in LLVM, a lot of the TODOs and patterns to emulate C++17 features can be eliminated. The steps I have taken were essentially: ``` git grep C++17 git grep c++17 git grep "initializer_list<int>" ``` and address given comments and patterns. Most of the changes boiled down to just using fold expressions rather than initializer_list. While doing this I also discovered that Clang by default restricts the depth of fold expressions to 256 elements. I specifically hit this with `TestDialect` in `addOperations`. I opted to not replace it with fold expressions because of that but instead adding a comment documenting the issue. If any other functions may be called with more than 256 elements in the future we might have to revert other parts as well. I don't think this is a common occurence besides the `TestDialect` however. If need be, this could potentially be fixed via `mlir-tblgen` in the future. Differential Revision: https://reviews.llvm.org/D131323
1 parent f0f1bca commit 26d811b

File tree

19 files changed

+55
-108
lines changed

19 files changed

+55
-108
lines changed

mlir/examples/standalone/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ project(standalone-dialect LANGUAGES CXX C)
33

44
set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
55

6-
set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
6+
set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
77

88
find_package(MLIR REQUIRED CONFIG)
99

mlir/include/mlir/Dialect/Bufferization/IR/BufferizableOpInterface.h

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -52,19 +52,16 @@ class OpFilter {
5252
template <typename... DialectTs>
5353
void allowDialect() {
5454
// The following expands a call to allowDialectImpl for each dialect
55-
// in 'DialectTs'. This magic is necessary due to a limitation in the places
56-
// that a parameter pack can be expanded in c++11.
57-
// FIXME: In c++17 this can be simplified by using 'fold expressions'.
58-
(void)std::initializer_list<int>{0, (allowDialectImpl<DialectTs>(), 0)...};
55+
// in 'DialectTs'.
56+
(allowDialectImpl<DialectTs>(), ...);
5957
}
6058

6159
/// Deny the given dialects.
6260
///
6361
/// This function adds one or multiple DENY entries.
6462
template <typename... DialectTs>
6563
void denyDialect() {
66-
// FIXME: In c++17 this can be simplified by using 'fold expressions'.
67-
(void)std::initializer_list<int>{0, (denyDialectImpl<DialectTs>(), 0)...};
64+
(denyDialectImpl<DialectTs>(), ...);
6865
}
6966

7067
/// Allow the given dialect.
@@ -82,17 +79,15 @@ class OpFilter {
8279
/// This function adds one or multiple ALLOW entries.
8380
template <typename... OpTys>
8481
void allowOperation() {
85-
// FIXME: In c++17 this can be simplified by using 'fold expressions'.
86-
(void)std::initializer_list<int>{0, (allowOperationImpl<OpTys>(), 0)...};
82+
(allowOperationImpl<OpTys>(), ...);
8783
}
8884

8985
/// Deny the given ops.
9086
///
9187
/// This function adds one or multiple DENY entries.
9288
template <typename... OpTys>
9389
void denyOperation() {
94-
// FIXME: In c++17 this can be simplified by using 'fold expressions'.
95-
(void)std::initializer_list<int>{0, (denyOperationImpl<OpTys>(), 0)...};
90+
(denyOperationImpl<OpTys>(), ...);
9691
}
9792

9893
/// Allow the given op.

mlir/include/mlir/Dialect/Transform/IR/TransformDialect.td

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -334,13 +334,10 @@ def Transform_Dialect : Dialect {
334334
/// dialect. Checks that they implement the required interfaces.
335335
template <typename... OpTys>
336336
void addOperationsChecked() {
337-
(void)std::initializer_list<int>{(addOperationIfNotRegistered<OpTys>(),
338-
0)...};
337+
(addOperationIfNotRegistered<OpTys>(),...);
339338

340339
#ifndef NDEBUG
341-
(void)std::initializer_list<int>{
342-
(detail::checkImplementsTransformInterface<OpTys>(getContext()),
343-
0)...};
340+
(detail::checkImplementsTransformInterface<OpTys>(getContext()),...);
344341
#endif // NDEBUG
345342
}
346343

mlir/include/mlir/ExecutionEngine/ExecutionEngine.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,9 +172,7 @@ class ExecutionEngine {
172172
llvm::SmallVector<void *> argsArray;
173173
// Pack every arguments in an array of pointers. Delegate the packing to a
174174
// trait so that it can be overridden per argument type.
175-
// TODO: replace with a fold expression when migrating to C++17.
176-
int dummy[] = {0, ((void)Argument<Args>::pack(argsArray, args), 0)...};
177-
(void)dummy;
175+
(Argument<Args>::pack(argsArray, args), ...);
178176
return invokePacked(adapterName, argsArray);
179177
}
180178

mlir/include/mlir/IR/BuiltinAttributes.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,6 @@ class DenseElementsAttr : public Attribute {
8484
/// Type trait used to check if the given type T is a potentially valid C++
8585
/// floating point type that can be used to access the underlying element
8686
/// types of a DenseElementsAttr.
87-
// TODO: Use std::disjunction when C++17 is supported.
8887
template <typename T>
8988
struct is_valid_cpp_fp_type {
9089
/// The type is a valid floating point type if it is a builtin floating

mlir/include/mlir/IR/BuiltinTypes.td

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ def Builtin_Integer : Builtin_Type<"Integer"> {
262262

263263
/// Integer representation maximal bitwidth.
264264
/// Note: This is aligned with the maximum width of llvm::IntegerType.
265-
static constexpr unsigned kMaxWidth = (1 << 24) - 1;
265+
static constexpr inline unsigned kMaxWidth = (1 << 24) - 1;
266266
}];
267267
}
268268

mlir/include/mlir/IR/Dialect.h

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -186,8 +186,7 @@ class Dialect {
186186
/// Register a set of dialect interfaces with this dialect instance.
187187
template <typename... Args>
188188
void addInterfaces() {
189-
(void)std::initializer_list<int>{
190-
0, (addInterface(std::make_unique<Args>(this)), 0)...};
189+
(addInterface(std::make_unique<Args>(this)), ...);
191190
}
192191
template <typename InterfaceT, typename... Args>
193192
InterfaceT &addInterface(Args &&...args) {
@@ -210,14 +209,18 @@ class Dialect {
210209
///
211210
template <typename... Args>
212211
void addOperations() {
212+
// This initializer_list argument pack expansion is essentially equal to
213+
// using a fold expression with a comma operator. Clang however, refuses
214+
// to compile a fold expression with a depth of more than 256 by default.
215+
// There seem to be no such limitations for initializer_list.
213216
(void)std::initializer_list<int>{
214217
0, (RegisteredOperationName::insert<Args>(*this), 0)...};
215218
}
216219

217220
/// Register a set of type classes with this dialect.
218221
template <typename... Args>
219222
void addTypes() {
220-
(void)std::initializer_list<int>{0, (addType<Args>(), 0)...};
223+
(addType<Args>(), ...);
221224
}
222225

223226
/// Register a type instance with this dialect.
@@ -228,7 +231,7 @@ class Dialect {
228231
/// Register a set of attribute classes with this dialect.
229232
template <typename... Args>
230233
void addAttributes() {
231-
(void)std::initializer_list<int>{0, (addAttribute<Args>(), 0)...};
234+
(addAttribute<Args>(), ...);
232235
}
233236

234237
/// Register an attribute instance with this dialect.

mlir/include/mlir/IR/DialectRegistry.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,8 +175,7 @@ class DialectRegistry {
175175
/// Add the given extensions to the registry.
176176
template <typename... ExtensionsT>
177177
void addExtensions() {
178-
(void)std::initializer_list<int>{
179-
(addExtension(std::make_unique<ExtensionsT>()), 0)...};
178+
(addExtension(std::make_unique<ExtensionsT>()), ...);
180179
}
181180

182181
/// Add an extension function that requires the given dialects.

mlir/include/mlir/IR/Matchers.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -224,10 +224,9 @@ struct PatternMatcherValue {
224224
template <typename TupleT, class CallbackT, std::size_t... Is>
225225
constexpr void enumerateImpl(TupleT &&tuple, CallbackT &&callback,
226226
std::index_sequence<Is...>) {
227-
(void)std::initializer_list<int>{
228-
0,
229-
(callback(std::integral_constant<std::size_t, Is>{}, std::get<Is>(tuple)),
230-
0)...};
227+
228+
(callback(std::integral_constant<std::size_t, Is>{}, std::get<Is>(tuple)),
229+
...);
231230
}
232231

233232
template <typename... Tys, typename CallbackT>

mlir/include/mlir/IR/OpDefinition.h

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1492,12 +1492,10 @@ using has_fold_trait =
14921492
template <typename T>
14931493
using detect_has_fold_trait = llvm::is_detected<has_fold_trait, T>;
14941494
/// Trait to check if T provides any `foldTrait` method.
1495-
/// NOTE: This should use std::disjunction when C++17 is available.
14961495
template <typename T>
14971496
using detect_has_any_fold_trait =
1498-
std::conditional_t<bool(detect_has_fold_trait<T>::value),
1499-
detect_has_fold_trait<T>,
1500-
detect_has_single_result_fold_trait<T>>;
1497+
std::disjunction<detect_has_fold_trait<T>,
1498+
detect_has_single_result_fold_trait<T>>;
15011499

15021500
/// Returns the result of folding a trait that implements a `foldTrait` function
15031501
/// that is specialized for operations that have a single result.
@@ -1543,10 +1541,7 @@ foldTrait(Operation *, ArrayRef<Attribute>, SmallVectorImpl<OpFoldResult> &) {
15431541
template <typename... Ts>
15441542
static LogicalResult foldTraits(Operation *op, ArrayRef<Attribute> operands,
15451543
SmallVectorImpl<OpFoldResult> &results) {
1546-
bool anyFolded = false;
1547-
(void)std::initializer_list<int>{
1548-
(anyFolded |= succeeded(foldTrait<Ts>(op, operands, results)), 0)...};
1549-
return success(anyFolded);
1544+
return success((succeeded(foldTrait<Ts>(op, operands, results)) | ...));
15501545
}
15511546

15521547
//===----------------------------------------------------------------------===//
@@ -1581,10 +1576,7 @@ verifyTrait(Operation *) {
15811576
/// Given a set of traits, return the result of verifying the given operation.
15821577
template <typename... Ts>
15831578
LogicalResult verifyTraits(Operation *op) {
1584-
LogicalResult result = success();
1585-
(void)std::initializer_list<int>{
1586-
(result = succeeded(result) ? verifyTrait<Ts>(op) : failure(), 0)...};
1587-
return result;
1579+
return success((succeeded(verifyTrait<Ts>(op)) && ...));
15881580
}
15891581

15901582
/// Verify the given trait if it provides a region verifier.
@@ -1604,12 +1596,7 @@ verifyRegionTrait(Operation *) {
16041596
/// given operation.
16051597
template <typename... Ts>
16061598
LogicalResult verifyRegionTraits(Operation *op) {
1607-
(void)op;
1608-
LogicalResult result = success();
1609-
(void)std::initializer_list<int>{
1610-
(result = succeeded(result) ? verifyRegionTrait<Ts>(op) : failure(),
1611-
0)...};
1612-
return result;
1599+
return success((succeeded(verifyRegionTrait<Ts>(op)) && ...));
16131600
}
16141601
} // namespace op_definition_impl
16151602

@@ -1695,7 +1682,7 @@ class Op : public OpState, public Traits<ConcreteType>... {
16951682
llvm::report_fatal_error(
16961683
"Attempting to attach an interface to an unregistered operation " +
16971684
ConcreteType::getOperationName() + ".");
1698-
(void)std::initializer_list<int>{(checkInterfaceTarget<Models>(), 0)...};
1685+
(checkInterfaceTarget<Models>(), ...);
16991686
info->attachInterface<Models...>();
17001687
}
17011688

0 commit comments

Comments
 (0)