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

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

mlir/include/mlir/IR/PatternMatch.h

Lines changed: 19 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1079,15 +1079,10 @@ LogicalResult verifyAsArgs(PatternRewriter &rewriter, ArrayRef<PDLValue> values,
10791079
auto errorFn = [&](const Twine &msg) {
10801080
return rewriter.notifyMatchFailure(rewriter.getUnknownLoc(), msg);
10811081
};
1082-
LogicalResult result = success();
1083-
(void)std::initializer_list<int>{
1084-
(result =
1085-
succeeded(result)
1086-
? ProcessPDLValue<typename FnTraitsT::template arg_t<I + 1>>::
1087-
verifyAsArg(errorFn, values[I], I)
1088-
: failure(),
1089-
0)...};
1090-
return result;
1082+
return success(
1083+
(succeeded(ProcessPDLValue<typename FnTraitsT::template arg_t<I + 1>>::
1084+
verifyAsArg(errorFn, values[I], I)) &&
1085+
...));
10911086
}
10921087

10931088
/// Assert that the given PDLValues match the constraints defined by the
@@ -1102,10 +1097,10 @@ void assertArgs(PatternRewriter &rewriter, ArrayRef<PDLValue> values,
11021097
auto errorFn = [&](const Twine &msg) -> LogicalResult {
11031098
llvm::report_fatal_error(msg);
11041099
};
1105-
(void)std::initializer_list<int>{
1106-
(assert(succeeded(ProcessPDLValue<typename FnTraitsT::template arg_t<
1107-
I + 1>>::verifyAsArg(errorFn, values[I], I))),
1108-
0)...};
1100+
(assert(succeeded(
1101+
ProcessPDLValue<typename FnTraitsT::template arg_t<I + 1>>::verifyAsArg(
1102+
errorFn, values[I], I))),
1103+
...);
11091104
#endif
11101105
}
11111106

@@ -1134,10 +1129,7 @@ template <typename... Ts>
11341129
static void processResults(PatternRewriter &rewriter, PDLResultList &results,
11351130
std::tuple<Ts...> &&tuple) {
11361131
auto applyFn = [&](auto &&...args) {
1137-
// TODO: Use proper fold expressions when we have C++17. For now we use a
1138-
// bogus std::initializer_list to work around C++14 limitations.
1139-
(void)std::initializer_list<int>{
1140-
(processResults(rewriter, results, std::move(args)), 0)...};
1132+
(processResults(rewriter, results, std::move(args)), ...);
11411133
};
11421134
llvm::apply_tuple(applyFn, std::move(tuple));
11431135
}
@@ -1412,14 +1404,10 @@ class RewritePatternSet {
14121404
typename = std::enable_if_t<sizeof...(Ts) != 0>>
14131405
RewritePatternSet &add(ConstructorArg &&arg, ConstructorArgs &&...args) {
14141406
// The following expands a call to emplace_back for each of the pattern
1415-
// types 'Ts'. This magic is necessary due to a limitation in the places
1416-
// that a parameter pack can be expanded in c++11.
1417-
// FIXME: In c++17 this can be simplified by using 'fold expressions'.
1418-
(void)std::initializer_list<int>{
1419-
0, (addImpl<Ts>(/*debugLabels=*/llvm::None,
1420-
std::forward<ConstructorArg>(arg),
1421-
std::forward<ConstructorArgs>(args)...),
1422-
0)...};
1407+
// types 'Ts'.
1408+
(addImpl<Ts>(/*debugLabels=*/llvm::None, std::forward<ConstructorArg>(arg),
1409+
std::forward<ConstructorArgs>(args)...),
1410+
...);
14231411
return *this;
14241412
}
14251413
/// An overload of the above `add` method that allows for attaching a set
@@ -1433,19 +1421,16 @@ class RewritePatternSet {
14331421
ConstructorArg &&arg,
14341422
ConstructorArgs &&...args) {
14351423
// The following expands a call to emplace_back for each of the pattern
1436-
// types 'Ts'. This magic is necessary due to a limitation in the places
1437-
// that a parameter pack can be expanded in c++11.
1438-
// FIXME: In c++17 this can be simplified by using 'fold expressions'.
1439-
(void)std::initializer_list<int>{
1440-
0, (addImpl<Ts>(debugLabels, arg, args...), 0)...};
1424+
// types 'Ts'.
1425+
(addImpl<Ts>(debugLabels, arg, args...), ...);
14411426
return *this;
14421427
}
14431428

14441429
/// Add an instance of each of the pattern types 'Ts'. Return a reference to
14451430
/// `this` for chaining insertions.
14461431
template <typename... Ts>
14471432
RewritePatternSet &add() {
1448-
(void)std::initializer_list<int>{0, (addImpl<Ts>(), 0)...};
1433+
(addImpl<Ts>(), ...);
14491434
return *this;
14501435
}
14511436

@@ -1498,19 +1483,16 @@ class RewritePatternSet {
14981483
typename = std::enable_if_t<sizeof...(Ts) != 0>>
14991484
RewritePatternSet &insert(ConstructorArg &&arg, ConstructorArgs &&...args) {
15001485
// The following expands a call to emplace_back for each of the pattern
1501-
// types 'Ts'. This magic is necessary due to a limitation in the places
1502-
// that a parameter pack can be expanded in c++11.
1503-
// FIXME: In c++17 this can be simplified by using 'fold expressions'.
1504-
(void)std::initializer_list<int>{
1505-
0, (addImpl<Ts>(/*debugLabels=*/llvm::None, arg, args...), 0)...};
1486+
// types 'Ts'.
1487+
(addImpl<Ts>(/*debugLabels=*/llvm::None, arg, args...), ...);
15061488
return *this;
15071489
}
15081490

15091491
/// Add an instance of each of the pattern types 'Ts'. Return a reference to
15101492
/// `this` for chaining insertions.
15111493
template <typename... Ts>
15121494
RewritePatternSet &insert() {
1513-
(void)std::initializer_list<int>{0, (addImpl<Ts>(), 0)...};
1495+
(addImpl<Ts>(), ...);
15141496
return *this;
15151497
}
15161498

mlir/include/mlir/IR/StorageUniquerSupport.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,8 @@ class StorageUserBase : public BaseT, public Traits<ConcreteT>... {
138138
if (!abstract)
139139
llvm::report_fatal_error("Registering an interface for an attribute/type "
140140
"that is not itself registered.");
141-
(void)std::initializer_list<int>{
142-
(checkInterfaceTarget<IfaceModels>(), 0)...};
141+
142+
(checkInterfaceTarget<IfaceModels>(), ...);
143143
abstract->interfaceMap.template insert<IfaceModels...>();
144144
}
145145

mlir/include/mlir/Support/InterfaceSupport.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -191,16 +191,14 @@ class InterfaceMap {
191191
/// do not represent interfaces are not added to the interface map.
192192
template <typename... Types>
193193
static InterfaceMap get() {
194-
// TODO: Use constexpr if here in C++17.
195194
constexpr size_t numInterfaces = num_interface_types_t<Types...>::value;
196-
if (numInterfaces == 0)
195+
if constexpr (numInterfaces == 0)
197196
return InterfaceMap();
198197

199198
std::array<std::pair<TypeID, void *>, numInterfaces> elements;
200199
std::pair<TypeID, void *> *elementIt = elements.data();
201200
(void)elementIt;
202-
(void)std::initializer_list<int>{
203-
0, (addModelAndUpdateIterator<Types>(elementIt), 0)...};
201+
(addModelAndUpdateIterator<Types>(elementIt), ...);
204202
return InterfaceMap(elements);
205203
}
206204

mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -178,15 +178,12 @@ struct AlignedAllocOpLowering : public AllocLikeOpLLVMLowering {
178178
}
179179

180180
/// The minimum alignment to use with aligned_alloc (has to be a power of 2).
181-
static constexpr uint64_t kMinAlignedAllocAlignment = 16UL;
181+
static inline constexpr uint64_t kMinAlignedAllocAlignment = 16UL;
182182

183183
/// Default layout to use in absence of the corresponding analysis.
184184
DataLayout defaultLayout;
185185
};
186186

187-
// Out of line definition, required till C++17.
188-
constexpr uint64_t AlignedAllocOpLowering::kMinAlignedAllocAlignment;
189-
190187
struct AllocaOpLowering : public AllocLikeOpLLVMLowering {
191188
AllocaOpLowering(LLVMTypeConverter &converter)
192189
: AllocLikeOpLLVMLowering(memref::AllocaOp::getOperationName(),

mlir/lib/Dialect/Linalg/IR/LinalgDialect.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,7 @@ void addNamedOpBuilderImpl(
9696
template <typename... OpTypes>
9797
void addNamedOpBuilders(
9898
llvm::StringMap<LinalgDialect::RegionBuilderFunType> &map) {
99-
(void)std::initializer_list<int>{0,
100-
(addNamedOpBuilderImpl<OpTypes>(map), 0)...};
99+
(addNamedOpBuilderImpl<OpTypes>(map), ...);
101100
}
102101

103102
void mlir::linalg::LinalgDialect::initialize() {

mlir/lib/Dialect/Linalg/Transforms/BufferizableOpInterfaceImpl.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,8 +137,7 @@ struct LinalgOpInterface
137137
template <typename... Ops>
138138
struct LinalgOpInterfaceHelper {
139139
static void registerOpInterface(MLIRContext *ctx) {
140-
(void)std::initializer_list<int>{
141-
0, (Ops::template attachInterface<LinalgOpInterface<Ops>>(*ctx), 0)...};
140+
(Ops::template attachInterface<LinalgOpInterface<Ops>>(*ctx), ...);
142141
}
143142
};
144143
} // namespace

0 commit comments

Comments
 (0)