Skip to content

[CIR] Implement folder for VecShuffleDynamicOp #142315

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 5, 2025
Merged

[CIR] Implement folder for VecShuffleDynamicOp #142315

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
12c9d3f
[CIR] Implement folder for VecShuffleDynamicOp
AmrDeveloper Jun 1, 2025
959bf9c
Address code review comments
AmrDeveloper Jun 2, 2025
ec7d646
Use getAsRange in for range
AmrDeveloper Jun 2, 2025
2a58fc9
Extend the folding test
AmrDeveloper Jun 4, 2025
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
1 change: 1 addition & 0 deletions clang/include/clang/CIR/Dialect/IR/CIROps.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2192,6 +2192,7 @@ def VecShuffleDynamicOp : CIR_Op<"vec.shuffle.dynamic",
}];

let hasVerifier = 1;
let hasFolder = 1;
}

#endif // CLANG_CIR_DIALECT_IR_CIROPS_TD
32 changes: 32 additions & 0 deletions clang/lib/CIR/Dialect/IR/CIRDialect.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "clang/CIR/Dialect/IR/CIROpsDialect.cpp.inc"
#include "clang/CIR/Dialect/IR/CIROpsEnums.cpp.inc"
#include "clang/CIR/MissingFeatures.h"

#include <numeric>

using namespace mlir;
Expand Down Expand Up @@ -1579,6 +1580,37 @@ OpFoldResult cir::VecExtractOp::fold(FoldAdaptor adaptor) {
// VecShuffleDynamicOp
//===----------------------------------------------------------------------===//

OpFoldResult cir::VecShuffleDynamicOp::fold(FoldAdaptor adaptor) {
mlir::Attribute vec = adaptor.getVec();
mlir::Attribute indices = adaptor.getIndices();
if (mlir::isa_and_nonnull<cir::ConstVectorAttr>(vec) &&
mlir::isa_and_nonnull<cir::ConstVectorAttr>(indices)) {
auto vecAttr = mlir::cast<cir::ConstVectorAttr>(vec);
auto indicesAttr = mlir::cast<cir::ConstVectorAttr>(indices);
auto vecTy = mlir::cast<cir::VectorType>(vecAttr.getType());

mlir::ArrayAttr vecElts = vecAttr.getElts();
mlir::ArrayAttr indicesElts = indicesAttr.getElts();

const uint64_t numElements = vecElts.size();

SmallVector<mlir::Attribute, 16> elements;
elements.reserve(numElements);

const uint64_t maskBits = llvm::NextPowerOf2(numElements - 1) - 1;
for (const auto &idxAttr : indicesElts.getAsRange<cir::IntAttr>()) {
uint64_t idxValue = idxAttr.getUInt();
uint64_t newIdx = idxValue & maskBits;
elements.push_back(vecElts[newIdx]);
}

return cir::ConstVectorAttr::get(
vecTy, mlir::ArrayAttr::get(getContext(), elements));
}

return {};
Copy link
Collaborator

@erichkeane erichkeane Jun 2, 2025

Choose a reason for hiding this comment

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

Should this branch have a 'not yet implemented' thing here?

Copy link
Member Author

@AmrDeveloper AmrDeveloper Jun 2, 2025

Choose a reason for hiding this comment

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

As far as I understood, no, it should be null because that means we can't fold VecShuffleDynamicOp with non Const vec operands

}

LogicalResult cir::VecShuffleDynamicOp::verify() {
// The number of elements in the two input vectors must match.
if (getVec().getType().getSize() !=
Expand Down
6 changes: 3 additions & 3 deletions clang/lib/CIR/Dialect/Transforms/CIRCanonicalize.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,10 @@ void CIRCanonicalizePass::runOnOperation() {
assert(!cir::MissingFeatures::complexRealOp());
assert(!cir::MissingFeatures::complexImagOp());
assert(!cir::MissingFeatures::callOp());
// CastOp, UnaryOp and VecExtractOp are here to perform a manual `fold` in
// applyOpPatternsGreedily.
// CastOp, UnaryOp, VecExtractOp and VecShuffleDynamicOp are here to perform
// a manual `fold` in applyOpPatternsGreedily.
if (isa<BrOp, BrCondOp, CastOp, ScopeOp, SwitchOp, SelectOp, UnaryOp,
VecExtractOp>(op))
VecExtractOp, VecShuffleDynamicOp>(op))
ops.push_back(op);
});

Expand Down
18 changes: 18 additions & 0 deletions clang/test/CIR/Transforms/vector-shuffle-dynamic-fold.cir
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// RUN: cir-opt %s -cir-canonicalize -o - | FileCheck %s

!s32i = !cir.int<s, 32>

module {
cir.func @fold_shuffle_dynamic_vector_op_test() {
%alloca = cir.alloca !cir.vector<4 x !s32i>, !cir.ptr<!cir.vector<4 x !s32i>>, ["r", init]
%vec = cir.const #cir.const_vector<[#cir.int<1> : !s32i, #cir.int<2> : !s32i, #cir.int<3> : !s32i, #cir.int<4> : !s32i]> : !cir.vector<4 x !s32i>
%indices = cir.const #cir.const_vector<[#cir.int<8> : !s32i, #cir.int<7> : !s32i, #cir.int<6> : !s32i, #cir.int<5> : !s32i]> : !cir.vector<4 x !s32i>
%new_vec = cir.vec.shuffle.dynamic %vec : !cir.vector<4 x !s32i>, %indices : !cir.vector<4 x !s32i>
cir.store align(16) %new_vec, %alloca : !cir.vector<4 x !s32i>, !cir.ptr<!cir.vector<4 x !s32i>>
cir.return
}

// CHECK: %[[NEW_VEC:.*]] = cir.const #cir.const_vector<[#cir.int<1> : !s32i, #cir.int<4> : !s32i, #cir.int<3> : !s32i, #cir.int<2> : !s32i]> : !cir.vector<4 x !s32i>
Copy link
Member

@bcardosolopes bcardosolopes Jun 4, 2025

Choose a reason for hiding this comment

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

Please change the testcase to return the vector instead of storing it and have a CHECK line that checks the return value is NEW_VEC.

Copy link
Contributor

@andykaylor andykaylor Jun 4, 2025

Choose a reason for hiding this comment

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

It's not obvious to me from this check what the full form of the transformed function will be. I assume we're getting rid of some dead instructions here, but that's not clear from the check. I'd suggest showing the full transformed function with CHECK-NEXT to verify that no unnecessary artifacts were left behind.

It isn't obvious at all why "8, 7, 6, 5" yielded the result here. I see that those values are ANDed with 3 to use "0, 3, 2, 1" as the shuffle indexes. Maybe add another test with indexes that don't require the mask and add a comment explaining the masking here.

}