Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions mlir/lib/Dialect/Affine/Utils/Utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,27 @@ class AffineApplyExpander
/// let remainder = srem a, b;
/// negative = a < 0 in
/// select negative, remainder + b, remainder.
///
/// Special case for power of 2: use bitwise AND (x & (n-1)) for non-negative
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

power of 2 RHS

/// x.
Value visitModExpr(AffineBinaryOpExpr expr) {
if (auto rhsConst = dyn_cast<AffineConstantExpr>(expr.getRHS())) {
if (rhsConst.getValue() <= 0) {
emitError(loc, "modulo by non-positive value is not supported");
return nullptr;
}

// Special case: x mod n where n is a power of 2 can be optimized to x &
// (n-1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Terminate all comments with a full stop. LLVM style.

int64_t rhsValue = rhsConst.getValue();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this assignment to above L88 to avoid multiple calls to getValue().

if (rhsValue > 0 && (rhsValue & (rhsValue - 1)) == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we've already returned for all rhs values <= 0 at L90. rhsValue is guaranteed to be positive now. This check is unnecessary.

auto lhs = visit(expr.getLHS());
assert(lhs && "unexpected affine expr lowering failure");

Value maskCst =
builder.create<arith::ConstantIndexOp>(loc, rhsValue - 1);
return builder.create<arith::AndIOp>(loc, lhs, maskCst);
}
}

auto lhs = visit(expr.getLHS());
Expand Down
9 changes: 9 additions & 0 deletions mlir/test/Conversion/AffineToStandard/lower-affine.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -927,3 +927,12 @@ func.func @affine_parallel_with_reductions_i64(%arg0: memref<3x3xi64>, %arg1: me
// CHECK: scf.reduce.return %[[RES]] : i64
// CHECK: }
// CHECK: }

#map_mod_8 = affine_map<(i) -> (i mod 8)>
// CHECK-LABEL: func @affine_apply_mod_8
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also check if using ... mod 1 doesn't lead to any unexpected behavior.

func.func @affine_apply_mod_8(%arg0 : index) -> (index) {
// CHECK-NEXT: %[[c7:.*]] = arith.constant 7 : index
// CHECK-NEXT: %[[v0:.*]] = arith.andi %arg0, %[[c7]] : index
%0 = affine.apply #map_mod_8 (%arg0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can put the map inline for better readability.

return %0 : index
}