Skip to content

Commit dd8aa3d

Browse files
bcardosolopesbcardosolopes
committed
Address review feedback
1 parent eff046f commit dd8aa3d

File tree

4 files changed

+44
-29
lines changed

4 files changed

+44
-29
lines changed

‎mlir/include/mlir/Dialect/LLVMIR/LLVMDialectBytecode.td‎

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ class EnumClassFlag<string flag, string getter> :
5757
// LLVMAttrDefs.td
5858
// - The mnemonics are either LLVM or builtin MLIR attributes and types, but
5959
// regular C++ types are also allowed to match builders and parsers.
60+
// - DIScopeAttr and DINodeAttr are empty base classes, custom encoding not
61+
// needed.
6062
//===----------------------------------------------------------------------===//
6163

6264
//===----------------------------------------------------------------------===//
@@ -107,7 +109,7 @@ def DIFileAttr : DialectAttribute<(attr
107109
//===----------------------------------------------------------------------===//
108110

109111
def DILocalVariableAttr : DialectAttribute<(attr
110-
Attr<"DIScopeAttr">:$scope, // Non-optional attribute
112+
Attr<"DIScopeAttr">:$scope,
111113
OptionalAttribute<"StringAttr">:$name,
112114
OptionalAttribute<"DIFileAttr">:$file,
113115
VarInt:$line,
@@ -158,7 +160,7 @@ def DISubprogramAttr : DialectAttribute<(attr
158160
Bool:$isRecSelf,
159161
OptionalAttribute<"DistinctAttr">:$id,
160162
OptionalAttribute<"DICompileUnitAttr">:$compileUnit,
161-
OptionalAttribute<"DIScopeAttr">:$scope, // TODO: DIScopeAttr
163+
OptionalAttribute<"DIScopeAttr">:$scope,
162164
OptionalAttribute<"StringAttr">:$name,
163165
OptionalAttribute<"StringAttr">:$linkageName,
164166
OptionalAttribute<"DIFileAttr">:$file,
@@ -207,7 +209,7 @@ def DIDerivedTypeAttr : DialectAttribute<(attr
207209
VarInt:$alignInBits,
208210
VarInt:$offsetInBits,
209211
OptionalInt<"unsigned">:$dwarfAddressSpace,
210-
OptionalAttribute<"DINodeAttr">:$extraData // TODO: DINodeAttr
212+
OptionalAttribute<"DINodeAttr">:$extraData
211213
)>;
212214

213215
//===----------------------------------------------------------------------===//
@@ -305,26 +307,27 @@ def DISubrangeAttr : DialectAttribute<(attr
305307

306308
def LoopAnnotationAttr : DialectAttribute<(attr
307309
OptionalAttribute<"BoolAttr">:$disableNonforced,
308-
OptionalAttribute<"LoopVectorizeAttr">:$vectorize, // TODO: LoopVectorizeAttr
309-
OptionalAttribute<"LoopInterleaveAttr">:$interleave, // TODO: LoopInterleaveAttr
310-
OptionalAttribute<"LoopUnrollAttr">:$unroll, // TODO: LoopUnrollAttr
311-
OptionalAttribute<"LoopUnrollAndJamAttr">:$unrollAndJam, // TODO: LoopUnrollAndJamAttr
312-
OptionalAttribute<"LoopLICMAttr">:$licm, // TODO: LoopLICMAttr
313-
OptionalAttribute<"LoopDistributeAttr">:$distribute, // TODO: LoopDistributeAttr
314-
OptionalAttribute<"LoopPipelineAttr">:$pipeline, // TODO: LoopPipelineAttr
315-
OptionalAttribute<"LoopPeeledAttr">:$peeled, // TODO: LoopPeeledAttr
316-
OptionalAttribute<"LoopUnswitchAttr">:$unswitch, // TODO: LoopUnswitchAttr
310+
OptionalAttribute<"LoopVectorizeAttr">:$vectorize,
311+
OptionalAttribute<"LoopInterleaveAttr">:$interleave,
312+
OptionalAttribute<"LoopUnrollAttr">:$unroll,
313+
OptionalAttribute<"LoopUnrollAndJamAttr">:$unrollAndJam,
314+
OptionalAttribute<"LoopLICMAttr">:$licm,
315+
OptionalAttribute<"LoopDistributeAttr">:$distribute,
316+
OptionalAttribute<"LoopPipelineAttr">:$pipeline,
317+
OptionalAttribute<"LoopPeeledAttr">:$peeled,
318+
OptionalAttribute<"LoopUnswitchAttr">:$unswitch,
317319
OptionalAttribute<"BoolAttr">:$mustProgress,
318320
OptionalAttribute<"BoolAttr">:$isVectorized,
319321
OptionalAttribute<"FusedLoc">:$startLoc,
320322
OptionalAttribute<"FusedLoc">:$endLoc,
321-
OptionalArrayRef<"AccessGroupAttr">:$parallelAccesses // TODO: AccessGroupAttr
323+
OptionalArrayRef<"AccessGroupAttr">:$parallelAccesses
322324
)>;
323325

324326
//===----------------------------------------------------------------------===//
325327
// Attributes & Types with custom bytecode handling.
326328
//===----------------------------------------------------------------------===//
327329

330+
// All the attributes with custom bytecode handling.
328331
def LLVMDialectAttributes : DialectAttributes<"LLVM"> {
329332
let elems = [
330333
AliasScopeAttr,
@@ -347,6 +350,10 @@ def LLVMDialectAttributes : DialectAttributes<"LLVM"> {
347350
DISubrangeAttr,
348351
DISubroutineTypeAttr,
349352
LoopAnnotationAttr
353+
// Referenced attributes currently missing support:
354+
// AccessGroupAttr, LoopVectorizeAttr, LoopInterleaveAttr, LoopUnrollAttr,
355+
// LoopUnrollAndJamAttr, LoopLICMAttr, LoopDistributeAttr, LoopPipelineAttr,
356+
// LoopPeeledAttr, LoopUnswitchAttr
350357
];
351358
}
352359

‎mlir/lib/Dialect/LLVMIR/IR/LLVMDialectBytecode.cpp‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ static LogicalResult writeAttribute(Attribute attribute,
3333
// Optional ArrayRefs
3434
//
3535
// Note that both the writer and reader functions consider attributes to be
36-
// optional. This is because the attribute may be present by empty.
36+
// optional. This is because the attribute may be present or empty.
3737
//===--------------------------------------------------------------------===//
3838

3939
template <class EntryTy>
@@ -64,7 +64,7 @@ static LogicalResult readOptionalArrayRef(DialectBytecodeReader &reader,
6464
if (!isPresent)
6565
return success();
6666

67-
auto readOperations = [&]() -> FailureOr<EntryTy> {
67+
auto readEntry = [&]() -> FailureOr<EntryTy> {
6868
EntryTy temp;
6969
if constexpr (std::is_base_of_v<Attribute, EntryTy>) {
7070
if (succeeded(reader.readOptionalAttribute(temp)))
@@ -77,7 +77,7 @@ static LogicalResult readOptionalArrayRef(DialectBytecodeReader &reader,
7777
return failure();
7878
};
7979

80-
return reader.readList(storage, readOperations);
80+
return reader.readList(storage, readEntry);
8181
}
8282

8383
//===--------------------------------------------------------------------===//
@@ -103,7 +103,7 @@ static LogicalResult readOptionalInt(DialectBytecodeReader &reader,
103103
if (failed(reader.readVarIntWithFlag(result, flag)))
104104
return failure();
105105
if (flag)
106-
storage = (decltype(storage.value()))result;
106+
storage = static_cast<EntryTy>(result);
107107
else
108108
storage = std::nullopt;
109109
return success();
@@ -124,9 +124,7 @@ struct LLVMDialectBytecodeInterface : public BytecodeDialectInterface {
124124
LLVMDialectBytecodeInterface(Dialect *dialect)
125125
: BytecodeDialectInterface(dialect) {}
126126

127-
//===--------------------------------------------------------------------===//
128127
// Attributes
129-
130128
Attribute readAttribute(DialectBytecodeReader &reader) const override {
131129
return ::readAttribute(getContext(), reader);
132130
}
@@ -136,9 +134,7 @@ struct LLVMDialectBytecodeInterface : public BytecodeDialectInterface {
136134
return ::writeAttribute(attr, writer);
137135
}
138136

139-
//===--------------------------------------------------------------------===//
140137
// Types
141-
142138
Type readType(DialectBytecodeReader &reader) const override {
143139
return ::readType(getContext(), reader);
144140
}

‎mlir/lib/Dialect/LLVMIR/IR/LLVMDialectBytecode.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
//
77
//===----------------------------------------------------------------------===//
88
//
9-
// This header defines hooks into the LLVMization dialect bytecode
9+
// This header defines hooks into the LLVM dialect bytecode
1010
// implementation.
1111
//
1212
//===----------------------------------------------------------------------===//
@@ -18,8 +18,8 @@ namespace mlir::LLVM {
1818
class LLVMDialect;
1919

2020
namespace detail {
21-
/// Add the interfaces necessary for encoding the LLVMization dialect
22-
/// components in bytecode.
21+
/// Add the interfaces necessary for encoding the LLVM dialect components in
22+
/// bytecode.
2323
void addBytecodeInterface(LLVMDialect *dialect);
2424
} // namespace detail
2525
} // namespace mlir::LLVM

‎mlir/test/Dialect/LLVMIR/bytecode.mlir‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
#loc6 = loc(fused<#di_subprogram>[#loc1])
99
#loc7 = loc(fused<#di_subprogram>[#loc2])
1010
#loop_annotation = #llvm.loop_annotation<disableNonforced = false, mustProgress = true, startLoc = #loc6, endLoc = #loc7, parallelAccesses = #access_group, #access_group1>
11+
#alias_scope_domain = #llvm.alias_scope_domain<id = distinct[0]<>, description = "The domain">
12+
#alias_scope = #llvm.alias_scope<id = distinct[0]<>, domain = #alias_scope_domain, description = "The domain">
1113
module {
1214
llvm.func @imp_fn() {
1315
llvm.return loc(#loc2)
@@ -17,6 +19,10 @@ module {
1719
^bb1: // pred: ^bb0
1820
llvm.return loc(#loc5)
1921
} loc(#loc3)
22+
llvm.func @experimental_noalias_scope_decl() {
23+
llvm.intr.experimental.noalias.scope.decl #alias_scope
24+
llvm.return
25+
}
2026
} loc(#loc)
2127
#di_file = #llvm.di_file<"test.f90" in "">
2228
#di_subroutine_type = #llvm.di_subroutine_type<callingConvention = DW_CC_program>
@@ -36,9 +42,11 @@ module {
3642

3743
// CHECK: #access_group = #llvm.access_group<id = distinct[0]<>>
3844
// CHECK: #access_group1 = #llvm.access_group<id = distinct[1]<>>
39-
// CHECK: #di_subprogram = #llvm.di_subprogram<recId = distinct[2]<>>
45+
// CHECK: #alias_scope_domain = #llvm.alias_scope_domain<id = distinct[2]<>, description = "The domain">
46+
// CHECK: #di_subprogram = #llvm.di_subprogram<recId = distinct[3]<>>
4047
// CHECK: {{.*}} = loc("test.f90":12:14)
4148
// CHECK: {{.*}} = loc("test":4:3)
49+
// CHECK: #alias_scope = #llvm.alias_scope<id = distinct[4]<>, domain = #alias_scope_domain, description = "The domain">
4250
// CHECK: {{.*}} = loc(fused<#di_subprogram>[{{.*}}])
4351
// CHECK: {{.*}} = loc(fused<#di_subprogram>[{{.*}}])
4452
// CHECK: #loop_annotation = #llvm.loop_annotation<disableNonforced = false, mustProgress = true, startLoc = {{.*}}, endLoc = {{.*}}, parallelAccesses = #access_group, #access_group1>
@@ -51,19 +59,23 @@ module {
5159
// CHECK: ^bb1: // pred: ^bb0
5260
// CHECK: llvm.return loc({{.*}})
5361
// CHECK: } loc({{.*}})
62+
// CHECK: llvm.func @experimental_noalias_scope_decl() {
63+
// CHECK: llvm.intr.experimental.noalias.scope.decl #alias_scope loc({{.*}})
64+
// CHECK: llvm.return loc({{.*}})
65+
// CHECK: } loc({{.*}})
5466
// CHECK: } loc({{.*}})
5567
// CHECK: #di_file = #llvm.di_file<"test.f90" in "">
5668
// CHECK: #di_subroutine_type = #llvm.di_subroutine_type<callingConvention = DW_CC_program>
5769
// CHECK: {{.*}} = loc("test":0:0)
5870
// CHECK: {{.*}} = loc("test-path":36:3)
5971
// CHECK: {{.*}} = loc("test-path":37:5)
6072
// CHECK: {{.*}} = loc("test-path":39:5)
61-
// CHECK: #di_compile_unit = #llvm.di_compile_unit<id = distinct[3]<>, sourceLanguage = DW_LANG_Fortran95, file = #di_file, isOptimized = false, emissionKind = Full>
62-
// CHECK: #di_compile_unit1 = #llvm.di_compile_unit<id = distinct[4]<>, sourceLanguage = DW_LANG_Fortran95, file = #di_file, isOptimized = false, emissionKind = Full>
63-
// CHECK: #di_compile_unit2 = #llvm.di_compile_unit<id = distinct[5]<>, sourceLanguage = DW_LANG_Fortran95, file = #di_file, isOptimized = false, emissionKind = Full>
73+
// CHECK: #di_compile_unit = #llvm.di_compile_unit<id = distinct[5]<>, sourceLanguage = DW_LANG_Fortran95, file = #di_file, isOptimized = false, emissionKind = Full>
74+
// CHECK: #di_compile_unit1 = #llvm.di_compile_unit<id = distinct[6]<>, sourceLanguage = DW_LANG_Fortran95, file = #di_file, isOptimized = false, emissionKind = Full>
75+
// CHECK: #di_compile_unit2 = #llvm.di_compile_unit<id = distinct[7]<>, sourceLanguage = DW_LANG_Fortran95, file = #di_file, isOptimized = false, emissionKind = Full>
6476
// CHECK: #di_module = #llvm.di_module<file = #di_file, scope = #di_compile_unit1, name = "mod1">
6577
// CHECK: #di_module1 = #llvm.di_module<file = #di_file, scope = #di_compile_unit2, name = "mod2">
6678
// CHECK: #di_imported_entity = #llvm.di_imported_entity<tag = DW_TAG_imported_module, scope = #di_subprogram, entity = #di_module, file = #di_file, line = 1>
6779
// CHECK: #di_imported_entity1 = #llvm.di_imported_entity<tag = DW_TAG_imported_module, scope = #di_subprogram, entity = #di_module1, file = #di_file, line = 1>
68-
// CHECK: #di_subprogram1 = #llvm.di_subprogram<recId = distinct[2]<>, id = distinct[6]<>, compileUnit = #di_compile_unit, scope = #di_file, name = "imp_fn", file = #di_file, subprogramFlags = Definition, type = #di_subroutine_type, retainedNodes = #di_imported_entity, #di_imported_entity1>
80+
// CHECK: #di_subprogram1 = #llvm.di_subprogram<recId = distinct[3]<>, id = distinct[8]<>, compileUnit = #di_compile_unit, scope = #di_file, name = "imp_fn", file = #di_file, subprogramFlags = Definition, type = #di_subroutine_type, retainedNodes = #di_imported_entity, #di_imported_entity1>
6981
// CHECK: {{.*}} = loc(fused<#di_subprogram1>[{{.*}}])

0 commit comments

Comments
 (0)