Skip to content

Commit 374d5da

Browse files
authored
[MLIR][Interfaces] Remove negative branch weight verifier (#148234)
This commit removes the verifier that checked if branch weights are negative. This check was too strict because weights are interpreted as unsigned integers. This showed up when running the verifier on LLVM dialect modules that were imported from LLVM IR.
1 parent a6494a3 commit 374d5da

File tree

4 files changed

+16
-37
lines changed

4 files changed

+16
-37
lines changed

flang/test/Fir/invalid.fir

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1426,12 +1426,3 @@ func.func @wrong_weights_number_in_if_then_else(%cond: i1) {
14261426
}
14271427
return
14281428
}
1429-
1430-
// -----
1431-
1432-
func.func @negative_weight_in_if_then(%cond: i1) {
1433-
// expected-error @below {{weight #0 must be non-negative}}
1434-
fir.if %cond weights([-1, 101]) {
1435-
}
1436-
return
1437-
}

mlir/include/mlir/Interfaces/ControlFlowInterfaces.td

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -384,13 +384,15 @@ def WeightedBranchOpInterface : OpInterface<"WeightedBranchOpInterface"> {
384384
This interface provides weight information for branching terminator
385385
operations, i.e. terminator operations with successors.
386386

387-
This interface provides methods for getting/setting integer non-negative
388-
weight of each branch. The probability of executing a branch
389-
is computed as the ratio between the branch's weight and the total
390-
sum of the weights (which cannot be zero).
391-
The weights are optional. If they are provided, then their number
387+
This interface provides methods for getting/setting integer weights of each
388+
branch. The probability of executing a branch is computed as the ratio
389+
between the branch's weight and the total sum of the weights (which cannot
390+
be zero). The weights are optional. If they are provided, then their number
392391
must match the number of successors of the operation.
393392

393+
Note that the branch weight use an i32 representation but they are to be
394+
interpreted as unsigned integers.
395+
394396
The default implementations of the methods expect the operation
395397
to have an attribute of type DenseI32ArrayAttr named branch_weights.
396398
}];
@@ -440,19 +442,21 @@ def WeightedRegionBranchOpInterface
440442
This interface provides weight information for region operations
441443
that exhibit branching behavior between held regions.
442444

443-
This interface provides methods for getting/setting integer non-negative
444-
weight of each branch. The probability of executing a region is computed
445-
as the ratio between the region branch's weight and the total sum
446-
of the weights (which cannot be zero).
447-
The weights are optional. If they are provided, then their number
448-
must match the number of regions held by the operation
449-
(including empty regions).
445+
This interface provides methods for getting/setting integer weights of each
446+
branch. The probability of executing a region is computed as the ratio
447+
between the region branch's weight and the total sum of the weights (which
448+
cannot be zero). The weights are optional. If they are provided, then their
449+
number must match the number of regions held by the operation (including
450+
empty regions).
450451

451452
The weights specify the probability of branching to a particular
452453
region when first executing the operation.
453454
For example, for loop-like operations with a single region
454455
the weight specifies the probability of entering the loop.
455456

457+
Note that the branch weight use an i32 representation but they are to be
458+
interpreted as unsigned integers.
459+
456460
The default implementations of the methods expect the operation
457461
to have an attribute of type DenseI32ArrayAttr named branch_weights.
458462
}];

mlir/lib/Interfaces/ControlFlowInterfaces.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,6 @@ static LogicalResult verifyWeights(Operation *op,
9999
<< ": " << weights.size() << " vs "
100100
<< expectedWeightsNum;
101101

102-
for (auto [index, weight] : llvm::enumerate(weights))
103-
if (weight < 0)
104-
return op->emitError() << "weight #" << index << " must be non-negative";
105-
106102
if (llvm::all_of(weights, [](int32_t value) { return value == 0; }))
107103
return op->emitError() << "branch weights cannot all be zero";
108104

mlir/test/Dialect/ControlFlow/invalid.mlir

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -82,18 +82,6 @@ func.func @wrong_weights_number(%cond: i1) {
8282

8383
// -----
8484

85-
// CHECK-LABEL: func @negative_weight
86-
func.func @wrong_total_weight(%cond: i1) {
87-
// expected-error@+1 {{weight #0 must be non-negative}}
88-
cf.cond_br %cond weights([-1, 101]), ^bb1, ^bb2
89-
^bb1:
90-
return
91-
^bb2:
92-
return
93-
}
94-
95-
// -----
96-
9785
// CHECK-LABEL: func @zero_weights
9886
func.func @wrong_total_weight(%cond: i1) {
9987
// expected-error@+1 {{branch weights cannot all be zero}}

0 commit comments

Comments
 (0)