Skip to content

[CIR] Upstream comparison ops for VectorType #140597

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 3 commits into from
May 23, 2025
Merged

[CIR] Upstream comparison ops for VectorType #140597

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
be154f1
[CIR] Upstream comparion ops for VectorType
AmrDeveloper May 18, 2025
7edb2e1
Address code review comments
AmrDeveloper May 20, 2025
7b9b955
Add test cases for unsigned and floating-point
AmrDeveloper May 21, 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
29 changes: 29 additions & 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 @@ -2105,4 +2105,33 @@ def VecExtractOp : CIR_Op<"vec.extract", [Pure,
let hasFolder = 1;
}

//===----------------------------------------------------------------------===//
// VecCmpOp
//===----------------------------------------------------------------------===//

def VecCmpOp : CIR_Op<"vec.cmp", [Pure, SameTypeOperands]> {
Copy link
Contributor

@andykaylor andykaylor May 19, 2025

Choose a reason for hiding this comment

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

Is there a reason we can't just have cir.cmp work with vector types?

Copy link
Member

@bcardosolopes bcardosolopes May 20, 2025

Choose a reason for hiding this comment

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

A new operation was created rather than reusing cir.cmp because the result is a vector of a signed integral type, not a bool.

Copy link
Contributor

@andykaylor andykaylor May 20, 2025

Choose a reason for hiding this comment

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

I guess that makes sense based on the C/C++ language handling, but I see that the classic codegen generates a vector of i1 and then sign-extends it to the size of the elements that were compared. and we lower to that same pattern when we go to the LLVM dialect.

This is fine for now, but I think we should consider using cir.cmp + cir.cast(bool_to_int) later.

Copy link
Member

@bcardosolopes bcardosolopes May 21, 2025

Choose a reason for hiding this comment

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

classic codegen generates a vector of i1 and then sign-extends it to the size of the elements that were compared

Right, but it does that extension within each lane (https://godbolt.org/z/3Ybez5cv5). CmpOp result is CIR_BoolType which has no relation with the operands. This is different from VecCmpOp, where the result matches the operands. If we were to make CmpOp more flexible, the operation would have to skip constraints and defer to a hand written verifier (and maybe more?) - this is also fine, but in these situations is just more common in MLIR to create a new op.

I think we should consider using cir.cmp + cir.cast(bool_to_int) later.

What advantages do you think this might bring to CIR? It only seems to create more indirections when analyzing vector comparisons. IMO this is fine as a LLVM lowering details.


let summary = "Compare two vectors";
let description = [{
The `cir.vec.cmp` operation does an element-wise comparison of two vectors
of the same type. The result is a vector of the same size as the operands
whose element type is the signed integral type that is the same size as the
element type of the operands. The values in the result are 0 or -1.

```mlir
%eq = cir.vec.cmp(eq, %vec_a, %vec_b) : !cir.vector<4 x !s32i>, !cir.vector<4 x !s32i>
%lt = cir.vec.cmp(lt, %vec_a, %vec_b) : !cir.vector<4 x !s32i>, !cir.vector<4 x !s32i>
```
}];

let arguments = (ins Arg<CmpOpKind, "cmp kind">:$kind, CIR_VectorType:$lhs,
CIR_VectorType:$rhs);
let results = (outs CIR_VectorType:$result);

let assemblyFormat = [{
`(` $kind `,` $lhs `,` $rhs `)` `:` qualified(type($lhs)) `,`
qualified(type($result)) attr-dict
}];
Copy link
Member

@bcardosolopes bcardosolopes May 20, 2025

Choose a reason for hiding this comment

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

Looks like we could get a folder here as well as follow up work at some point :)

}

#endif // CLANG_CIR_DIALECT_IR_CIROPS_TD
16 changes: 12 additions & 4 deletions clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -786,22 +786,30 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {
}
};

cir::CmpOpKind kind = clangCmpToCIRCmp(e->getOpcode());
if (lhsTy->getAs<MemberPointerType>()) {
assert(!cir::MissingFeatures::dataMemberType());
assert(e->getOpcode() == BO_EQ || e->getOpcode() == BO_NE);
mlir::Value lhs = cgf.emitScalarExpr(e->getLHS());
mlir::Value rhs = cgf.emitScalarExpr(e->getRHS());
cir::CmpOpKind kind = clangCmpToCIRCmp(e->getOpcode());
result = builder.createCompare(loc, kind, lhs, rhs);
} else if (!lhsTy->isAnyComplexType() && !rhsTy->isAnyComplexType()) {
BinOpInfo boInfo = emitBinOps(e);
mlir::Value lhs = boInfo.lhs;
mlir::Value rhs = boInfo.rhs;

if (lhsTy->isVectorType()) {
assert(!cir::MissingFeatures::vectorType());
cgf.cgm.errorNYI(loc, "vector comparisons");
result = builder.getBool(false, loc);
if (!e->getType()->isVectorType()) {
// If AltiVec, the comparison results in a numeric type, so we use
// intrinsics comparing vectors and giving 0 or 1 as a result
cgf.cgm.errorNYI(loc, "AltiVec comparison");
} else {
// Other kinds of vectors. Element-wise comparison returning
// a vector.
result = builder.create<cir::VecCmpOp>(
cgf.getLoc(boInfo.loc), cgf.convertType(boInfo.fullType), kind,
boInfo.lhs, boInfo.rhs);
}
} else if (boInfo.isFixedPointOp()) {
assert(!cir::MissingFeatures::fixedPointType());
cgf.cgm.errorNYI(loc, "fixed point comparisons");
Expand Down
32 changes: 31 additions & 1 deletion clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1676,7 +1676,8 @@ void ConvertCIRToLLVMPass::runOnOperation() {
CIRToLLVMUnaryOpLowering,
CIRToLLVMVecCreateOpLowering,
CIRToLLVMVecExtractOpLowering,
CIRToLLVMVecInsertOpLowering
CIRToLLVMVecInsertOpLowering,
CIRToLLVMVecCmpOpLowering
// clang-format on
>(converter, patterns.getContext());

Expand Down Expand Up @@ -1801,6 +1802,35 @@ mlir::LogicalResult CIRToLLVMVecInsertOpLowering::matchAndRewrite(
return mlir::success();
}

mlir::LogicalResult CIRToLLVMVecCmpOpLowering::matchAndRewrite(
cir::VecCmpOp op, OpAdaptor adaptor,
mlir::ConversionPatternRewriter &rewriter) const {
assert(mlir::isa<cir::VectorType>(op.getType()) &&
mlir::isa<cir::VectorType>(op.getLhs().getType()) &&
mlir::isa<cir::VectorType>(op.getRhs().getType()) &&
"Vector compare with non-vector type");
mlir::Type elementType = elementTypeIfVector(op.getLhs().getType());
mlir::Value bitResult;
if (auto intType = mlir::dyn_cast<cir::IntType>(elementType)) {
bitResult = rewriter.create<mlir::LLVM::ICmpOp>(
op.getLoc(),
convertCmpKindToICmpPredicate(op.getKind(), intType.isSigned()),
adaptor.getLhs(), adaptor.getRhs());
} else if (mlir::isa<cir::CIRFPTypeInterface>(elementType)) {
bitResult = rewriter.create<mlir::LLVM::FCmpOp>(
op.getLoc(), convertCmpKindToFCmpPredicate(op.getKind()),
adaptor.getLhs(), adaptor.getRhs());
} else {
return op.emitError() << "unsupported type for VecCmpOp: " << elementType;
}

// LLVM IR vector comparison returns a vector of i1. This one-bit vector
// must be sign-extended to the correct result type.
rewriter.replaceOpWithNewOp<mlir::LLVM::SExtOp>(
op, typeConverter->convertType(op.getType()), bitResult);
return mlir::success();
}

std::unique_ptr<mlir::Pass> createConvertCIRToLLVMPass() {
return std::make_unique<ConvertCIRToLLVMPass>();
}
Expand Down
10 changes: 10 additions & 0 deletions clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,16 @@ class CIRToLLVMVecInsertOpLowering
mlir::ConversionPatternRewriter &) const override;
};

class CIRToLLVMVecCmpOpLowering
: public mlir::OpConversionPattern<cir::VecCmpOp> {
public:
using mlir::OpConversionPattern<cir::VecCmpOp>::OpConversionPattern;

mlir::LogicalResult
matchAndRewrite(cir::VecCmpOp op, OpAdaptor,
mlir::ConversionPatternRewriter &) const override;
};

} // namespace direct
} // namespace cir

Expand Down
Loading