[MLIR][Vector] Add a missing builder implementation for Vector_WarpEx… #110106
Conversation
kurapov-peter
requested review from
banach-space,
dcaballe and
nicolasvasilache
as code owners
|
@llvm/pr-subscribers-mlir-vector @llvm/pr-subscribers-mlir Author: Petr Kurapov (kurapov-peter) Changes…ecuteOnLane0Op The op declares three builders: This adds the missing implementation for the first builder. Full diff: https://github.com/llvm/llvm-project/pull/110106.diff 1 Files Affected:
diff --git a/mlir/lib/Dialect/Vector/IR/VectorOps.cpp b/mlir/lib/Dialect/Vector/IR/VectorOps.cpp
index cac6b955457049..cf2b6e83f1fc0f 100644
--- a/mlir/lib/Dialect/Vector/IR/VectorOps.cpp
+++ b/mlir/lib/Dialect/Vector/IR/VectorOps.cpp
@@ -6421,6 +6421,12 @@ void WarpExecuteOnLane0Op::getSuccessorRegions(
regions.push_back(RegionSuccessor(&getWarpRegion()));
}
+void WarpExecuteOnLane0Op::build(OpBuilder &builder, OperationState &result,
+ Value laneId, int64_t warpSize) {
+ build(builder, result, TypeRange(), laneId, warpSize,
+ /*operands=*/std::nullopt, /*argTypes=*/std::nullopt);
+}
+
void WarpExecuteOnLane0Op::build(OpBuilder &builder, OperationState &result,
TypeRange resultTypes, Value laneId,
int64_t warpSize) {
|
MacDue
left a comment
MacDue
left a comment
There was a problem hiding this comment.
Maybe simply remove the builder given it must be unused?
I think it is unused because there's no implementation :) All uses I've seen so far pass an empty |
|
I agree with Ben. If the builder is not used, it should be deleted rather than implemented. If it is to be added, there should also be examples of it being used (these would demonstrate that this is indeed needed). As a rule of thumb, we should avoid implementing features that are not used - that's potential maintenance burden. While this is a rather small change, there's just no evidence of anyone needing it. Please correct me if I'm wrong :) |
|
Alright, I'm closing this |
llvm#112338) …ne0Op builder Removing the declaration instead of implementing the builder as discussed in llvm#110106
4 participants
…ecuteOnLane0Op
The op declares three builders:
This adds the missing implementation for the first builder.