-
Notifications
You must be signed in to change notification settings - Fork 14.6k
[CIR] Plus & Minus CompoundAssignment support for ComplexType #150759
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -108,6 +108,15 @@ class ComplexExprEmitter : public StmtVisitor<ComplexExprEmitter, mlir::Value> { | |||||||
|
||||||||
mlir::Value emitPromotedComplexOperand(const Expr *e, QualType promotionTy); | ||||||||
|
||||||||
LValue emitCompoundAssignLValue( | ||||||||
const CompoundAssignOperator *e, | ||||||||
mlir::Value (ComplexExprEmitter::*func)(const BinOpInfo &), | ||||||||
RValue &value); | ||||||||
|
||||||||
mlir::Value emitCompoundAssign( | ||||||||
const CompoundAssignOperator *e, | ||||||||
mlir::Value (ComplexExprEmitter::*func)(const BinOpInfo &)); | ||||||||
|
||||||||
mlir::Value emitBinAdd(const BinOpInfo &op); | ||||||||
mlir::Value emitBinSub(const BinOpInfo &op); | ||||||||
|
||||||||
|
@@ -143,6 +152,15 @@ class ComplexExprEmitter : public StmtVisitor<ComplexExprEmitter, mlir::Value> { | |||||||
HANDLEBINOP(Add) | ||||||||
HANDLEBINOP(Sub) | ||||||||
#undef HANDLEBINOP | ||||||||
|
||||||||
// Compound assignments. | ||||||||
mlir::Value VisitBinAddAssign(const CompoundAssignOperator *e) { | ||||||||
return emitCompoundAssign(e, &ComplexExprEmitter::emitBinAdd); | ||||||||
} | ||||||||
|
||||||||
mlir::Value VisitBinSubAssign(const CompoundAssignOperator *e) { | ||||||||
return emitCompoundAssign(e, &ComplexExprEmitter::emitBinSub); | ||||||||
} | ||||||||
}; | ||||||||
} // namespace | ||||||||
|
||||||||
|
@@ -153,6 +171,12 @@ static const ComplexType *getComplexType(QualType type) { | |||||||
return cast<ComplexType>(cast<AtomicType>(type)->getValueType()); | ||||||||
} | ||||||||
|
||||||||
static mlir::Value createComplexFromReal(CIRGenBuilderTy &builder, | ||||||||
mlir::Location loc, mlir::Value real) { | ||||||||
mlir::Value imag = builder.getNullValue(real.getType(), loc); | ||||||||
return builder.createComplexCreate(loc, real, imag); | ||||||||
} | ||||||||
|
||||||||
LValue ComplexExprEmitter::emitBinAssignLValue(const BinaryOperator *e, | ||||||||
mlir::Value &value) { | ||||||||
assert(cgf.getContext().hasSameUnqualifiedType(e->getLHS()->getType(), | ||||||||
|
@@ -570,6 +594,124 @@ ComplexExprEmitter::emitBinOps(const BinaryOperator *e, QualType promotionTy) { | |||||||
return binOpInfo; | ||||||||
} | ||||||||
|
||||||||
LValue ComplexExprEmitter::emitCompoundAssignLValue( | ||||||||
const CompoundAssignOperator *e, | ||||||||
mlir::Value (ComplexExprEmitter::*func)(const BinOpInfo &), RValue &value) { | ||||||||
QualType lhsTy = e->getLHS()->getType(); | ||||||||
QualType rhsTy = e->getRHS()->getType(); | ||||||||
SourceLocation exprLoc = e->getExprLoc(); | ||||||||
mlir::Location loc = cgf.getLoc(exprLoc); | ||||||||
|
||||||||
if (const AtomicType *atomicTy = lhsTy->getAs<AtomicType>()) | ||||||||
lhsTy = atomicTy->getValueType(); | ||||||||
|
||||||||
BinOpInfo opInfo{loc}; | ||||||||
opInfo.fpFeatures = e->getFPFeaturesInEffect(cgf.getLangOpts()); | ||||||||
|
||||||||
assert(!cir::MissingFeatures::cgFPOptionsRAII()); | ||||||||
|
||||||||
// Load the RHS and LHS operands. | ||||||||
// __block variables need to have the rhs evaluated first, plus this should | ||||||||
// improve codegen a little. | ||||||||
QualType promotionTypeCR = getPromotionType(e->getComputationResultType()); | ||||||||
opInfo.ty = promotionTypeCR.isNull() ? e->getComputationResultType() | ||||||||
: promotionTypeCR; | ||||||||
|
||||||||
QualType complexElementTy = | ||||||||
opInfo.ty->castAs<ComplexType>()->getElementType(); | ||||||||
QualType promotionTypeRHS = getPromotionType(rhsTy); | ||||||||
|
||||||||
// The RHS should have been converted to the computation type. | ||||||||
if (e->getRHS()->getType()->isRealFloatingType()) { | ||||||||
if (!promotionTypeRHS.isNull()) | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||||||||
opInfo.rhs = createComplexFromReal( | ||||||||
cgf.getBuilder(), loc, | ||||||||
cgf.emitPromotedScalarExpr(e->getRHS(), promotionTypeRHS)); | ||||||||
else { | ||||||||
assert(cgf.getContext().hasSameUnqualifiedType(complexElementTy, rhsTy)); | ||||||||
opInfo.rhs = createComplexFromReal(cgf.getBuilder(), loc, | ||||||||
cgf.emitScalarExpr(e->getRHS())); | ||||||||
} | ||||||||
} else { | ||||||||
if (!promotionTypeRHS.isNull()) { | ||||||||
opInfo.rhs = createComplexFromReal( | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we should be calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for catching this. I will update it here and in incubator |
||||||||
cgf.getBuilder(), loc, | ||||||||
cgf.emitPromotedComplexExpr(e->getRHS(), promotionTypeRHS)); | ||||||||
} else { | ||||||||
assert(cgf.getContext().hasSameUnqualifiedType(opInfo.ty, rhsTy)); | ||||||||
opInfo.rhs = Visit(e->getRHS()); | ||||||||
} | ||||||||
} | ||||||||
|
||||||||
LValue lhs = cgf.emitLValue(e->getLHS()); | ||||||||
|
||||||||
// Load from the l-value and convert it. | ||||||||
QualType promotionTypeLHS = getPromotionType(e->getComputationLHSType()); | ||||||||
if (lhsTy->isAnyComplexType()) { | ||||||||
mlir::Value lhsValue = emitLoadOfLValue(lhs, exprLoc); | ||||||||
QualType destTy = promotionTypeLHS.isNull() ? opInfo.ty : promotionTypeLHS; | ||||||||
opInfo.lhs = emitComplexToComplexCast(lhsValue, lhsTy, destTy, exprLoc); | ||||||||
} else { | ||||||||
mlir::Value lhsValue = cgf.emitLoadOfScalar(lhs, exprLoc); | ||||||||
andykaylor marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
// For floating point real operands we can directly pass the scalar form | ||||||||
// to the binary operator emission and potentially get more efficient code. | ||||||||
if (lhsTy->isRealFloatingType()) { | ||||||||
QualType promotedComplexElementTy; | ||||||||
if (!promotionTypeLHS.isNull()) { | ||||||||
promotedComplexElementTy = | ||||||||
cast<ComplexType>(promotionTypeLHS)->getElementType(); | ||||||||
if (!cgf.getContext().hasSameUnqualifiedType(promotedComplexElementTy, | ||||||||
promotionTypeLHS)) | ||||||||
lhsValue = cgf.emitScalarConversion( | ||||||||
lhsValue, lhsTy, promotedComplexElementTy, exprLoc); | ||||||||
} else { | ||||||||
if (!cgf.getContext().hasSameUnqualifiedType(complexElementTy, lhsTy)) | ||||||||
lhsValue = cgf.emitScalarConversion(lhsValue, lhsTy, complexElementTy, | ||||||||
exprLoc); | ||||||||
} | ||||||||
opInfo.lhs = createComplexFromReal(cgf.getBuilder(), | ||||||||
cgf.getLoc(e->getExprLoc()), lhsValue); | ||||||||
} else { | ||||||||
opInfo.lhs = emitScalarToComplexCast(lhsValue, lhsTy, opInfo.ty, exprLoc); | ||||||||
} | ||||||||
} | ||||||||
|
||||||||
// Expand the binary operator. | ||||||||
mlir::Value result = (this->*func)(opInfo); | ||||||||
|
||||||||
// Truncate the result and store it into the LHS lvalue. | ||||||||
if (lhsTy->isAnyComplexType()) { | ||||||||
mlir::Value resultValue = | ||||||||
emitComplexToComplexCast(result, opInfo.ty, lhsTy, exprLoc); | ||||||||
emitStoreOfComplex(loc, resultValue, lhs, /*isInit*/ false); | ||||||||
value = RValue::getComplex(resultValue); | ||||||||
} else { | ||||||||
mlir::Value resultValue = | ||||||||
cgf.emitComplexToScalarConversion(result, opInfo.ty, lhsTy, exprLoc); | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From the original CG, this part is for handling one case in C where the LHS is scalar and the RHS is complex. The code will not assert because the source is still complex llvm-project/clang/test/CodeGen/complex.c Line 34 in d4e8619
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you sure that gets to this code? LHS in that code should be 'cf' which will be a complex type, so it shouldn't fall through to here. Also, the result of the += expression in that case is also complex, and the line below stores a scalar result, not complex. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I selected the one line before the code that I want to show, in L35 llvm-project/clang/test/CodeGen/complex.c Line 35 in d4e8619
Src is Complex, destTy is double There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's NYI in the incubator, but I tested it with OCG llvm-project/clang/lib/CodeGen/CGExprComplex.cpp Line 1320 in bf7afe1
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it. Thanks! I didn't realize that was permitted in C. This is the case I asked about above that C++ didn't allow. Please add a test case for this also. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried multiple times to reproduce it with C++, but I realised in only valid in C I will add this test case and also check other OCG cases. Thanks |
||||||||
cgf.emitStoreOfScalar(resultValue, lhs, /*isInit*/ false); | ||||||||
value = RValue::get(resultValue); | ||||||||
} | ||||||||
|
||||||||
return lhs; | ||||||||
} | ||||||||
|
||||||||
mlir::Value ComplexExprEmitter::emitCompoundAssign( | ||||||||
const CompoundAssignOperator *e, | ||||||||
mlir::Value (ComplexExprEmitter::*func)(const BinOpInfo &)) { | ||||||||
RValue val; | ||||||||
LValue lv = emitCompoundAssignLValue(e, func, val); | ||||||||
|
||||||||
// The result of an assignment in C is the assigned r-value. | ||||||||
if (!cgf.getLangOpts().CPlusPlus) | ||||||||
return val.getComplexValue(); | ||||||||
|
||||||||
// If the lvalue is non-volatile, return the computed value of the assignment. | ||||||||
if (!lv.isVolatileQualified()) | ||||||||
return val.getComplexValue(); | ||||||||
|
||||||||
return emitLoadOfLValue(lv, e->getExprLoc()); | ||||||||
} | ||||||||
|
||||||||
mlir::Value ComplexExprEmitter::emitBinAdd(const BinOpInfo &op) { | ||||||||
assert(!cir::MissingFeatures::fastMathFlags()); | ||||||||
assert(!cir::MissingFeatures::cgFPOptionsRAII()); | ||||||||
|
@@ -600,6 +742,31 @@ mlir::Value CIRGenFunction::emitComplexExpr(const Expr *e) { | |||||||
return ComplexExprEmitter(*this).Visit(const_cast<Expr *>(e)); | ||||||||
} | ||||||||
|
||||||||
using CompoundFunc = | ||||||||
mlir::Value (ComplexExprEmitter::*)(const ComplexExprEmitter::BinOpInfo &); | ||||||||
|
||||||||
static CompoundFunc getComplexOp(BinaryOperatorKind Op) { | ||||||||
switch (Op) { | ||||||||
case BO_MulAssign: | ||||||||
llvm_unreachable("getComplexOp: BO_MulAssign"); | ||||||||
case BO_DivAssign: | ||||||||
llvm_unreachable("getComplexOp: BO_DivAssign"); | ||||||||
case BO_SubAssign: | ||||||||
return &ComplexExprEmitter::emitBinSub; | ||||||||
case BO_AddAssign: | ||||||||
return &ComplexExprEmitter::emitBinAdd; | ||||||||
default: | ||||||||
llvm_unreachable("unexpected complex compound assignment"); | ||||||||
} | ||||||||
} | ||||||||
|
||||||||
LValue CIRGenFunction::emitComplexCompoundAssignmentLValue( | ||||||||
const CompoundAssignOperator *e) { | ||||||||
CompoundFunc op = getComplexOp(e->getOpcode()); | ||||||||
RValue val; | ||||||||
return ComplexExprEmitter(*this).emitCompoundAssignLValue(e, op, val); | ||||||||
} | ||||||||
|
||||||||
mlir::Value CIRGenFunction::emitComplexPrePostIncDec(const UnaryOperator *e, | ||||||||
LValue lv, | ||||||||
cir::UnaryOpKind op, | ||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like this should be an
errorNYI
condition. I guess we'll hit an NYI diagnostic somewhere else, but this won't actually work. What I've been doing in cases like this is leaving the final code, but also adding anerrorNYI
call and a comment explaining that the code is correct but is expected to be NYI somewhere else.