Skip to content

[CIR] Implement AddOp for ComplexType #147578

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
Jul 10, 2025
+356 −0
Merged

[CIR] Implement AddOp for ComplexType #147578

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d91ac48
[CIR] Implement AddOp for ComplexType
AmrDeveloper Jul 8, 2025
29732d9
Address code review comments
AmrDeveloper Jul 9, 2025
e0b01d8
Add missing features asserts
AmrDeveloper Jul 9, 2025
7004089
Use replaceOpWithNewOp
AmrDeveloper Jul 10, 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
26 changes: 26 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 @@ -2521,6 +2521,32 @@ def ComplexImagOp : CIR_Op<"complex.imag", [Pure]> {
let hasFolder = 1;
}

//===----------------------------------------------------------------------===//
// ComplexAddOp
//===----------------------------------------------------------------------===//

def ComplexAddOp : CIR_Op<"complex.add", [Pure, SameOperandsAndResultType]> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we have separate operations for add/sub/mul/div or combine them into a single complex.binop? The strongest reason I can see for having them separate is that the 'complex' dialect has separate operations. On the other hand, the 'math' dialect has separate operations for add/sub/mul/div, but we still combined them in CIR. We should probably handle both the same way, and I'm inclined toward separate operations. I'm not sure why the decision was made to combine them for cir.binop.

@bcardosolopes What is your opinion?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am in favor of separate operations too, as it will simplify future optimizations, like specification of commutativity etc.

Sidenote: I would not incline to complex dialect that much, as it might get deprecated, since there is no maintainer still. See: https://discourse.llvm.org/t/mlir-project-maintainers/87189

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure why the decision was made to combine them for cir.binop. ... bcardosolopes What is your opinion?

I kept them combined for simplicity given all uses so far don't really require them to be separate (and lowering is still as simple as if they were separate). In hindsight using tablegen class for binop would have had similar effect.

Whenever it makes sense to split off cir.binop (commutativity spec, etc as mentioned by @xlauko, or just plain consistency), I'm in favor. In that spirit, I'd say let's keep complex separate to mirror the dialect.

Sidenote: I would not incline to complex dialect that much, as it might get deprecated, since there is no maintainer still. See: https://discourse.llvm.org/t/mlir-project-maintainers/87189

I had the same impression when I read this post. @andykaylor do any of your colleagues (or you) could be interested in maintaining the complex dialect?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll ask around. I feel like it is definitely useful, but I don't know who is actually using it.

let summary = "Complex addition";
let description = [{
The `cir.complex.add` operation takes two complex numbers and returns
their sum.

Example:

```mlir
%2 = cir.complex.add %0, %1 : !cir.complex<!cir.float>
```
}];

let arguments = (ins CIR_ComplexType:$lhs, CIR_ComplexType:$rhs);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we allow attaching integer overflow attributes to this operation? It looks like classic codegen doesn't do that, but I can't see why it shouldn't. This doesn't need to be done for this PR, but it's something we should think about.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, i think we should do that


let results = (outs CIR_ComplexType:$result);

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

//===----------------------------------------------------------------------===//
// Bit Manipulation Operations
//===----------------------------------------------------------------------===//
Expand Down
108 changes: 108 additions & 0 deletions clang/lib/CIR/CodeGen/CIRGenExprComplex.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,55 @@ class ComplexExprEmitter : public StmtVisitor<ComplexExprEmitter, mlir::Value> {
mlir::Value
VisitSubstNonTypeTemplateParmExpr(SubstNonTypeTemplateParmExpr *e);
mlir::Value VisitUnaryDeref(const Expr *e);

struct BinOpInfo {
mlir::Location loc;
mlir::Value lhs{};
mlir::Value rhs{};
QualType ty{}; // Computation Type.
FPOptions fpFeatures{};
};

BinOpInfo emitBinOps(const BinaryOperator *e,
QualType promotionTy = QualType());

mlir::Value emitPromoted(const Expr *e, QualType promotionTy);

mlir::Value emitPromotedComplexOperand(const Expr *e, QualType promotionTy);

mlir::Value emitBinAdd(const BinOpInfo &op);

QualType getPromotionType(QualType ty, bool isDivOpCode = false) {
if (auto *complexTy = ty->getAs<ComplexType>()) {
QualType elementTy = complexTy->getElementType();
if (isDivOpCode && elementTy->isFloatingType() &&
cgf.getLangOpts().getComplexRange() ==
LangOptions::ComplexRangeKind::CX_Promoted) {
cgf.cgm.errorNYI("HigherPrecisionTypeForComplexArithmetic");
return QualType();
}

if (elementTy.UseExcessPrecision(cgf.getContext()))
return cgf.getContext().getComplexType(cgf.getContext().FloatTy);
}

if (ty.UseExcessPrecision(cgf.getContext()))
return cgf.getContext().FloatTy;
return QualType();
}

#define HANDLEBINOP(OP) \
mlir::Value VisitBin##OP(const BinaryOperator *e) { \
QualType promotionTy = getPromotionType( \
e->getType(), e->getOpcode() == BinaryOperatorKind::BO_Div); \
mlir::Value result = emitBin##OP(emitBinOps(e, promotionTy)); \
if (!promotionTy.isNull()) \
cgf.cgm.errorNYI("Binop emitUnPromotedValue"); \
return result; \
}

HANDLEBINOP(Add)
#undef HANDLEBINOP
};
} // namespace

Expand Down Expand Up @@ -291,6 +340,60 @@ mlir::Value ComplexExprEmitter::VisitUnaryDeref(const Expr *e) {
return emitLoadOfLValue(e);
}

mlir::Value ComplexExprEmitter::emitPromoted(const Expr *e,
QualType promotionTy) {
e = e->IgnoreParens();
if (const auto *bo = dyn_cast<BinaryOperator>(e)) {
switch (bo->getOpcode()) {
#define HANDLE_BINOP(OP) \
case BO_##OP: \
return emitBin##OP(emitBinOps(bo, promotionTy));
HANDLE_BINOP(Add)
#undef HANDLE_BINOP
default:
break;
}
} else if (isa<UnaryOperator>(e)) {
cgf.cgm.errorNYI("emitPromoted UnaryOperator");
return {};
}

mlir::Value result = Visit(const_cast<Expr *>(e));
if (!promotionTy.isNull())
cgf.cgm.errorNYI("emitPromoted emitPromotedValue");

return result;
}

mlir::Value
ComplexExprEmitter::emitPromotedComplexOperand(const Expr *e,
QualType promotionTy) {
if (e->getType()->isAnyComplexType()) {
if (!promotionTy.isNull())
return cgf.emitPromotedComplexExpr(e, promotionTy);
return Visit(const_cast<Expr *>(e));
}

cgf.cgm.errorNYI("emitPromotedComplexOperand non-complex type");
return {};
}

ComplexExprEmitter::BinOpInfo
ComplexExprEmitter::emitBinOps(const BinaryOperator *e, QualType promotionTy) {
BinOpInfo binOpInfo{cgf.getLoc(e->getExprLoc())};
binOpInfo.lhs = emitPromotedComplexOperand(e->getLHS(), promotionTy);
binOpInfo.rhs = emitPromotedComplexOperand(e->getRHS(), promotionTy);
binOpInfo.ty = promotionTy.isNull() ? e->getType() : promotionTy;
binOpInfo.fpFeatures = e->getFPFeaturesInEffect(cgf.getLangOpts());
return binOpInfo;
}

mlir::Value ComplexExprEmitter::emitBinAdd(const BinOpInfo &op) {
assert(!cir::MissingFeatures::fastMathFlags());
assert(!cir::MissingFeatures::cgFPOptionsRAII());
return builder.create<cir::ComplexAddOp>(op.loc, op.lhs, op.rhs);
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll eventually want a way to attach fast-math flags or set exception handling and rounding mode constraints if this is a floating-point operation. Can you add missing features here for those?

}

LValue CIRGenFunction::emitComplexAssignmentLValue(const BinaryOperator *e) {
assert(e->getOpcode() == BO_Assign && "Expected assign op");

Expand All @@ -313,3 +416,8 @@ void CIRGenFunction::emitStoreOfComplex(mlir::Location loc, mlir::Value v,
LValue dest, bool isInit) {
ComplexExprEmitter(*this).emitStoreOfComplex(loc, v, dest, isInit);
}

mlir::Value CIRGenFunction::emitPromotedComplexExpr(const Expr *e,
QualType promotionType) {
return ComplexExprEmitter(*this).emitPromoted(e, promotionType);
}
2 changes: 2 additions & 0 deletions clang/lib/CIR/CodeGen/CIRGenFunction.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -886,6 +886,8 @@ class CIRGenFunction : public CIRGenTypeCache {
void emitInitializerForField(clang::FieldDecl *field, LValue lhs,
clang::Expr *init);

mlir::Value emitPromotedComplexExpr(const Expr *e, QualType promotionType);

mlir::Value emitPromotedScalarExpr(const Expr *e, QualType promotionType);

/// Emit the computation of the specified expression of scalar type.
Expand Down
50 changes: 50 additions & 0 deletions clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2048,6 +2048,7 @@ void ConvertCIRToLLVMPass::runOnOperation() {
CIRToLLVMBrOpLowering,
CIRToLLVMCallOpLowering,
CIRToLLVMCmpOpLowering,
CIRToLLVMComplexAddOpLowering,
CIRToLLVMComplexCreateOpLowering,
CIRToLLVMComplexImagOpLowering,
CIRToLLVMComplexRealOpLowering,
Expand Down Expand Up @@ -2357,6 +2358,55 @@ mlir::LogicalResult CIRToLLVMVecTernaryOpLowering::matchAndRewrite(
return mlir::success();
}

mlir::LogicalResult CIRToLLVMComplexAddOpLowering::matchAndRewrite(
cir::ComplexAddOp op, OpAdaptor adaptor,
mlir::ConversionPatternRewriter &rewriter) const {
mlir::Value lhs = adaptor.getLhs();
mlir::Value rhs = adaptor.getRhs();
mlir::Location loc = op.getLoc();

auto complexType = mlir::cast<cir::ComplexType>(op.getLhs().getType());
mlir::Type complexElemTy =
getTypeConverter()->convertType(complexType.getElementType());
auto lhsReal =
rewriter.create<mlir::LLVM::ExtractValueOp>(loc, complexElemTy, lhs, 0);
auto lhsImag =
rewriter.create<mlir::LLVM::ExtractValueOp>(loc, complexElemTy, lhs, 1);
auto rhsReal =
rewriter.create<mlir::LLVM::ExtractValueOp>(loc, complexElemTy, rhs, 0);
auto rhsImag =
rewriter.create<mlir::LLVM::ExtractValueOp>(loc, complexElemTy, rhs, 1);

mlir::Value newReal;
mlir::Value newImag;
if (complexElemTy.isInteger()) {
newReal = rewriter.create<mlir::LLVM::AddOp>(loc, complexElemTy, lhsReal,
rhsReal);
newImag = rewriter.create<mlir::LLVM::AddOp>(loc, complexElemTy, lhsImag,
rhsImag);
} else {
assert(!cir::MissingFeatures::fastMathFlags());
assert(!cir::MissingFeatures::fpConstraints());
newReal = rewriter.create<mlir::LLVM::FAddOp>(loc, complexElemTy, lhsReal,
Copy link
Contributor

Choose a reason for hiding this comment

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

This also needs fast-math and constrained FP math missing features.

rhsReal);
newImag = rewriter.create<mlir::LLVM::FAddOp>(loc, complexElemTy, lhsImag,
rhsImag);
}

mlir::Type complexLLVMTy =
getTypeConverter()->convertType(op.getResult().getType());
auto initialComplex =
rewriter.create<mlir::LLVM::PoisonOp>(op->getLoc(), complexLLVMTy);

auto realComplex = rewriter.create<mlir::LLVM::InsertValueOp>(
op->getLoc(), initialComplex, newReal, 0);

rewriter.replaceOpWithNewOp<mlir::LLVM::InsertValueOp>(op, realComplex,
newImag, 1);

return mlir::success();
}

mlir::LogicalResult CIRToLLVMComplexCreateOpLowering::matchAndRewrite(
cir::ComplexCreateOp op, OpAdaptor adaptor,
mlir::ConversionPatternRewriter &rewriter) const {
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 @@ -523,6 +523,16 @@ class CIRToLLVMGetBitfieldOpLowering
mlir::ConversionPatternRewriter &) const override;
};

class CIRToLLVMComplexAddOpLowering
: public mlir::OpConversionPattern<cir::ComplexAddOp> {
public:
using mlir::OpConversionPattern<cir::ComplexAddOp>::OpConversionPattern;

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

} // namespace direct
} // namespace cir

Expand Down
Loading