Skip to content

Commit bb1d976

Browse files
committed
[mlir][flang] use OpBuilder& instead of Builder* in <Op>::build methods
As we start defining more complex Ops, we increasingly see the need for Ops-with-regions to be able to construct Ops within their regions in their ::build methods. However, these methods only have access to Builder, and not OpBuilder. Creating a local instance of OpBuilder inside ::build and using it fails to trigger the operation creation hooks in derived builders (e.g., ConversionPatternRewriter). In this case, we risk breaking the logic of the derived builder. At the same time, OpBuilder::create, which is by far the largest user of ::build already passes "this" as the first argument, so an OpBuilder instance is already available. Update all ::build methods in all Ops in MLIR and Flang to take "OpBuilder &" instead of "Builder *". Note the change from pointer and to reference to comply with the common style in MLIR, this also ensures all other users must change their ::build methods. Differential Revision: https://reviews.llvm.org/D78713
1 parent 5c352e6 commit bb1d976

File tree

67 files changed

+630
-630
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

67 files changed

+630
-630
lines changed

flang/include/flang/Optimizer/Dialect/FIROps.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ class FirEndOp;
2121
class LoopOp;
2222
class RealAttr;
2323

24-
void buildCmpFOp(mlir::Builder *builder, mlir::OperationState &result,
24+
void buildCmpFOp(mlir::OpBuilder &builder, mlir::OperationState &result,
2525
mlir::CmpFPredicate predicate, mlir::Value lhs,
2626
mlir::Value rhs);
27-
void buildCmpCOp(mlir::Builder *builder, mlir::OperationState &result,
27+
void buildCmpCOp(mlir::OpBuilder &builder, mlir::OperationState &result,
2828
mlir::CmpFPredicate predicate, mlir::Value lhs,
2929
mlir::Value rhs);
3030
unsigned getCaseArgumentOffset(llvm::ArrayRef<mlir::Attribute> cases,

flang/include/flang/Optimizer/Dialect/FIROps.td

Lines changed: 39 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ class fir_SimpleOp<string mnemonic, list<OpTrait> traits>
140140

141141
// Base builder for allocate operations
142142
def fir_AllocateOpBuilder : OpBuilder<
143-
"Builder *builder, OperationState &result, Type inType,"
143+
"OpBuilder &builder, OperationState &result, Type inType,"
144144
"ValueRange lenParams = {}, ValueRange sizes = {},"
145145
"ArrayRef<NamedAttribute> attributes = {}",
146146
[{
@@ -151,19 +151,19 @@ def fir_AllocateOpBuilder : OpBuilder<
151151
}]>;
152152

153153
def fir_NamedAllocateOpBuilder : OpBuilder<
154-
"Builder *builder, OperationState &result, Type inType, StringRef name,"
154+
"OpBuilder &builder, OperationState &result, Type inType, StringRef name,"
155155
"ValueRange lenParams = {}, ValueRange sizes = {},"
156156
"ArrayRef<NamedAttribute> attributes = {}",
157157
[{
158158
result.addTypes(getRefTy(inType));
159159
result.addAttribute("in_type", TypeAttr::get(inType));
160-
result.addAttribute("name", builder->getStringAttr(name));
160+
result.addAttribute("name", builder.getStringAttr(name));
161161
result.addOperands(sizes);
162162
result.addAttributes(attributes);
163163
}]>;
164164

165165
def fir_OneResultOpBuilder : OpBuilder<
166-
"Builder *, OperationState &result, Type resultType,"
166+
"OpBuilder &, OperationState &result, Type resultType,"
167167
"ValueRange operands, ArrayRef<NamedAttribute> attributes = {}",
168168
[{
169169
if (resultType)
@@ -370,7 +370,7 @@ def fir_LoadOp : fir_OneResultOp<"load", [MemoryEffects<[MemRead]>]> {
370370
let arguments = (ins AnyReferenceLike:$memref);
371371

372372
let builders = [OpBuilder<
373-
"Builder *builder, OperationState &result, Value refVal",
373+
"OpBuilder &builder, OperationState &result, Value refVal",
374374
[{
375375
if (!refVal) {
376376
mlir::emitError(result.location, "LoadOp has null argument");
@@ -605,17 +605,17 @@ class fir_IntegralSwitchTerminatorOp<string mnemonic,
605605

606606
let skipDefaultBuilders = 1;
607607
let builders = [OpBuilder<
608-
"Builder *builder, OperationState &result, Value selector,"
608+
"OpBuilder &builder, OperationState &result, Value selector,"
609609
"ArrayRef<int64_t> compareOperands, ArrayRef<Block *> destinations,"
610610
"ArrayRef<ValueRange> destOperands = {},"
611611
"ArrayRef<NamedAttribute> attributes = {}",
612612
[{
613613
result.addOperands(selector);
614614
llvm::SmallVector<mlir::Attribute, 8> ivalues;
615615
for (auto iv : compareOperands)
616-
ivalues.push_back(builder->getI64IntegerAttr(iv));
617-
ivalues.push_back(builder->getUnitAttr());
618-
result.addAttribute(getCasesAttr(), builder->getArrayAttr(ivalues));
616+
ivalues.push_back(builder.getI64IntegerAttr(iv));
617+
ivalues.push_back(builder.getUnitAttr());
618+
result.addAttribute(getCasesAttr(), builder.getArrayAttr(ivalues));
619619
const auto count = destinations.size();
620620
for (auto d : destinations)
621621
result.addSuccessors(d);
@@ -633,9 +633,9 @@ class fir_IntegralSwitchTerminatorOp<string mnemonic,
633633
}
634634
}
635635
result.addAttribute(getOperandSegmentSizeAttr(),
636-
builder->getI32VectorAttr({1, 0, sumArgs}));
636+
builder.getI32VectorAttr({1, 0, sumArgs}));
637637
result.addAttribute(getTargetOffsetAttr(),
638-
builder->getI32VectorAttr(argOffs));
638+
builder.getI32VectorAttr(argOffs));
639639
result.addAttributes(attributes);
640640
}]
641641
>];
@@ -793,11 +793,11 @@ def fir_SelectCaseOp : fir_SwitchTerminatorOp<"select_case"> {
793793

794794
let skipDefaultBuilders = 1;
795795
let builders = [
796-
OpBuilder<"Builder *builder, OperationState &result, Value selector,"
796+
OpBuilder<"OpBuilder &builder, OperationState &result, Value selector,"
797797
"ArrayRef<mlir::Attribute> compareAttrs, ArrayRef<ValueRange> cmpOperands,"
798798
"ArrayRef<Block *> destinations, ArrayRef<ValueRange> destOperands = {},"
799799
"ArrayRef<NamedAttribute> attributes = {}">,
800-
OpBuilder<"Builder *builder, OperationState &result, Value selector,"
800+
OpBuilder<"OpBuilder &builder, OperationState &result, Value selector,"
801801
"ArrayRef<mlir::Attribute> compareAttrs, ArrayRef<Value> cmpOpList,"
802802
"ArrayRef<Block *> destinations, ArrayRef<ValueRange> destOperands = {},"
803803
"ArrayRef<NamedAttribute> attributes = {}">];
@@ -886,13 +886,13 @@ def fir_SelectTypeOp : fir_SwitchTerminatorOp<"select_type"> {
886886

887887
let skipDefaultBuilders = 1;
888888
let builders = [OpBuilder<
889-
"Builder *builder, OperationState &result, Value selector,"
889+
"OpBuilder &builder, OperationState &result, Value selector,"
890890
"ArrayRef<mlir::Attribute> typeOperands,"
891891
"ArrayRef<Block *> destinations, ArrayRef<ValueRange> destOperands = {},"
892892
"ArrayRef<NamedAttribute> attributes = {}",
893893
[{
894894
result.addOperands(selector);
895-
result.addAttribute(getCasesAttr(), builder->getArrayAttr(typeOperands));
895+
result.addAttribute(getCasesAttr(), builder.getArrayAttr(typeOperands));
896896
const auto count = destinations.size();
897897
for (auto d : destinations)
898898
result.addSuccessors(d);
@@ -910,9 +910,9 @@ def fir_SelectTypeOp : fir_SwitchTerminatorOp<"select_type"> {
910910
}
911911
}
912912
result.addAttribute(getOperandSegmentSizeAttr(),
913-
builder->getI32VectorAttr({1, 0, sumArgs}));
913+
builder.getI32VectorAttr({1, 0, sumArgs}));
914914
result.addAttribute(getTargetOffsetAttr(),
915-
builder->getI32VectorAttr(argOffs));
915+
builder.getI32VectorAttr(argOffs));
916916
result.addAttributes(attributes);
917917
}]
918918
>];
@@ -1596,10 +1596,10 @@ def fir_CoordinateOp : fir_Op<"coordinate_of", [NoSideEffect]> {
15961596

15971597
let skipDefaultBuilders = 1;
15981598
let builders = [
1599-
OpBuilder<"mlir::Builder *builder, OperationState &result,"
1599+
OpBuilder<"mlir::OpBuilder &builder, OperationState &result,"
16001600
"Type type, Value ref, ValueRange coor,"
16011601
"ArrayRef<NamedAttribute> attrs = {}">,
1602-
OpBuilder<"mlir::Builder *builder, OperationState &result,"
1602+
OpBuilder<"mlir::OpBuilder &builder, OperationState &result,"
16031603
"Type type, ValueRange operands,"
16041604
"ArrayRef<NamedAttribute> attrs = {}">];
16051605

@@ -1708,10 +1708,10 @@ def fir_FieldIndexOp : fir_OneResultOp<"field_index", [NoSideEffect]> {
17081708
}];
17091709

17101710
let builders = [OpBuilder<
1711-
"Builder *builder, OperationState &result, StringRef fieldName,"
1711+
"OpBuilder &builder, OperationState &result, StringRef fieldName,"
17121712
"Type recTy, ValueRange operands = {}",
17131713
[{
1714-
result.addAttribute(fieldAttrName(), builder->getStringAttr(fieldName));
1714+
result.addAttribute(fieldAttrName(), builder.getStringAttr(fieldName));
17151715
result.addAttribute(typeAttrName(), TypeAttr::get(recTy));
17161716
result.addOperands(operands);
17171717
}]
@@ -1832,9 +1832,9 @@ def fir_LenParamIndexOp : fir_OneResultOp<"len_param_index", [NoSideEffect]> {
18321832
}];
18331833

18341834
let builders = [OpBuilder<
1835-
"Builder *builder, OperationState &result, StringRef fieldName, Type recTy",
1835+
"OpBuilder &builder, OperationState &result, StringRef fieldName, Type recTy",
18361836
[{
1837-
result.addAttribute(fieldAttrName(), builder->getStringAttr(fieldName));
1837+
result.addAttribute(fieldAttrName(), builder.getStringAttr(fieldName));
18381838
result.addAttribute(typeAttrName(), TypeAttr::get(recTy));
18391839
}]
18401840
>];
@@ -1864,7 +1864,7 @@ def fir_ResultOp : fir_Op<"result", [NoSideEffect, ReturnLike, Terminator]> {
18641864

18651865
let arguments = (ins Variadic<AnyType>:$results);
18661866
let builders = [
1867-
OpBuilder<"Builder *builder, OperationState &result", "/* do nothing */">
1867+
OpBuilder<"OpBuilder &builder, OperationState &result", "/* do nothing */">
18681868
];
18691869

18701870
let assemblyFormat = "($results^ `:` type($results))? attr-dict";
@@ -1919,7 +1919,7 @@ def fir_LoopOp : region_Op<"do_loop",
19191919

19201920
let skipDefaultBuilders = 1;
19211921
let builders = [
1922-
OpBuilder<"mlir::Builder *builder, OperationState &result,"
1922+
OpBuilder<"mlir::OpBuilder &builder, OperationState &result,"
19231923
"mlir::Value lowerBound, mlir::Value upperBound,"
19241924
"mlir::Value step, bool unordered = false,"
19251925
"ValueRange iterArgs = llvm::None,"
@@ -1996,9 +1996,9 @@ def fir_WhereOp : region_Op<"if"> {
19961996

19971997
let skipDefaultBuilders = 1;
19981998
let builders = [
1999-
OpBuilder<"Builder *builder, OperationState &result, "
1999+
OpBuilder<"OpBuilder &builder, OperationState &result, "
20002000
"Value cond, bool withOtherRegion">,
2001-
OpBuilder<"Builder *builder, OperationState &result, "
2001+
OpBuilder<"OpBuilder &builder, OperationState &result, "
20022002
"TypeRange resultTypes, Value cond, bool withOtherRegion">
20032003
];
20042004

@@ -2043,7 +2043,7 @@ def fir_IterWhileOp : region_Op<"iterate_while",
20432043

20442044
let skipDefaultBuilders = 1;
20452045
let builders = [
2046-
OpBuilder<"mlir::Builder *builder, OperationState &result,"
2046+
OpBuilder<"mlir::OpBuilder &builder, OperationState &result,"
20472047
"mlir::Value lowerBound, mlir::Value upperBound,"
20482048
"mlir::Value step, mlir::Value iterate,"
20492049
"ValueRange iterArgs = llvm::None,"
@@ -2367,7 +2367,7 @@ def fir_CmpfOp : fir_Op<"cmpf",
23672367
let results = (outs AnyLogicalLike);
23682368

23692369
let builders = [OpBuilder<
2370-
"Builder *builder, OperationState &result, CmpFPredicate predicate,"
2370+
"OpBuilder &builder, OperationState &result, CmpFPredicate predicate,"
23712371
"Value lhs, Value rhs", [{
23722372
buildCmpFOp(builder, result, predicate, lhs, rhs);
23732373
}]>];
@@ -2476,7 +2476,7 @@ def fir_CmpcOp : fir_Op<"cmpc",
24762476
let printer = "printCmpcOp(p, *this);";
24772477

24782478
let builders = [OpBuilder<
2479-
"Builder *builder, OperationState &result, CmpFPredicate predicate,"
2479+
"OpBuilder &builder, OperationState &result, CmpFPredicate predicate,"
24802480
"Value lhs, Value rhs", [{
24812481
buildCmpCOp(builder, result, predicate, lhs, rhs);
24822482
}]>];
@@ -2608,7 +2608,7 @@ def fir_GenTypeDescOp : fir_OneResultOp<"gentypedesc", [NoSideEffect]> {
26082608
}];
26092609

26102610
let builders = [
2611-
OpBuilder<"Builder *, OperationState &result, mlir::TypeAttr inty">
2611+
OpBuilder<"OpBuilder &, OperationState &result, mlir::TypeAttr inty">
26122612
];
26132613

26142614
let verifier = [{
@@ -2713,23 +2713,23 @@ def fir_GlobalOp : fir_Op<"global", [IsolatedFromAbove, Symbol]> {
27132713

27142714
let skipDefaultBuilders = 1;
27152715
let builders = [
2716-
OpBuilder<"mlir::Builder *builder, OperationState &result,"
2716+
OpBuilder<"mlir::OpBuilder &builder, OperationState &result,"
27172717
"StringRef name, Type type, ArrayRef<NamedAttribute> attrs = {}">,
2718-
OpBuilder<"mlir::Builder *builder, OperationState &result,"
2718+
OpBuilder<"mlir::OpBuilder &builder, OperationState &result,"
27192719
"StringRef name, bool isConstant, Type type,"
27202720
"ArrayRef<NamedAttribute> attrs = {}">,
2721-
OpBuilder<"mlir::Builder *builder, OperationState &result,"
2721+
OpBuilder<"mlir::OpBuilder &builder, OperationState &result,"
27222722
"StringRef name, Type type, StringAttr linkage = {},"
27232723
"ArrayRef<NamedAttribute> attrs = {}">,
2724-
OpBuilder<"mlir::Builder *builder, OperationState &result,"
2724+
OpBuilder<"mlir::OpBuilder &builder, OperationState &result,"
27252725
"StringRef name, bool isConstant, Type type,"
27262726
"StringAttr linkage = {},"
27272727
"ArrayRef<NamedAttribute> attrs = {}">,
2728-
OpBuilder<"mlir::Builder *builder, OperationState &result,"
2728+
OpBuilder<"mlir::OpBuilder &builder, OperationState &result,"
27292729
"StringRef name, Type type, Attribute initVal,"
27302730
"StringAttr linkage = {},"
27312731
"ArrayRef<NamedAttribute> attrs = {}">,
2732-
OpBuilder<"mlir::Builder *builder, OperationState &result,"
2732+
OpBuilder<"mlir::OpBuilder &builder, OperationState &result,"
27332733
"StringRef name, bool isConstant, Type type,"
27342734
"Attribute initVal, StringAttr linkage = {},"
27352735
"ArrayRef<NamedAttribute> attrs = {}">,
@@ -2891,11 +2891,11 @@ def fir_DispatchTableOp : fir_Op<"dispatch_table",
28912891

28922892
let skipDefaultBuilders = 1;
28932893
let builders = [
2894-
OpBuilder<"mlir::Builder *builder, OperationState *result,"
2894+
OpBuilder<"mlir::OpBuilder &builder, OperationState *result,"
28952895
"StringRef name, Type type, ArrayRef<NamedAttribute> attrs = {}",
28962896
[{
28972897
result->addAttribute(mlir::SymbolTable::getSymbolAttrName(),
2898-
builder->getStringAttr(name));
2898+
builder.getStringAttr(name));
28992899
result->addAttributes(attrs);
29002900
}]>
29012901
];

0 commit comments

Comments
 (0)