[CLANG] Full support of complex multiplication and division. #81514
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -283,9 +283,46 @@ class ComplexExprEmitter | |
| ComplexPairTy EmitComplexBinOpLibCall(StringRef LibCallName, | ||
| const BinOpInfo &Op); | ||
|
|
||
| QualType HigherPrecisionTypeForComplexArithmetic(QualType ElementType, | ||
| bool IsDivOpCode) { | ||
| const TargetInfo &TI = CGF.getContext().getTargetInfo(); | ||
| const LangOptions Opts = CGF.getLangOpts(); | ||
| if (const auto *BT = dyn_cast<BuiltinType>(ElementType)) { | ||
| switch (BT->getKind()) { | ||
| case BuiltinType::Kind::Float16: { | ||
| if (TI.hasFloat16Type() && !TI.hasLegalHalfType()) | ||
| return CGF.getContext().getComplexType(CGF.getContext().FloatTy); | ||
| break; | ||
| } | ||
| case BuiltinType::Kind::BFloat16: { | ||
| if (TI.hasBFloat16Type() && !TI.hasFullBFloat16Type()) | ||
| return CGF.getContext().getComplexType(CGF.getContext().FloatTy); | ||
| break; | ||
| } | ||
| case BuiltinType::Kind::Float: | ||
| return CGF.getContext().getComplexType(CGF.getContext().DoubleTy); | ||
| break; | ||
| case BuiltinType::Kind::Double: { | ||
| if (TI.hasLongDoubleType()) | ||
| return CGF.getContext().getComplexType(CGF.getContext().LongDoubleTy); | ||
| return CGF.getContext().getComplexType(CGF.getContext().DoubleTy); | ||
|
There was a problem hiding this comment. I'm not sure it's a good idea to return a specific type here if it's not known to actually be higher precision? There was a problem hiding this comment. Yes, we need to check the sizes. If we "promote" from a 64-bit double to a 64-bit long double, we'll probably end up with something that gets optimized directly to the cx-limited-range ("basic") implementation. This can also be an issue for float. For example, AVR targets use a 32-bit type for both float and double. |
||
| break; | ||
| } | ||
| default: | ||
| return QualType(); | ||
| } | ||
| } | ||
| return QualType(); | ||
| } | ||
|
|
||
| QualType getPromotionType(QualType Ty, bool IsDivOpCode = false) { | ||
| if (auto *CT = Ty->getAs<ComplexType>()) { | ||
| QualType ElementType = CT->getElementType(); | ||
| if (IsDivOpCode && ElementType->isFloatingType() && | ||
| CGF.getLangOpts().getComplexRange() == | ||
| LangOptions::ComplexRangeKind::CX_Promoted) | ||
| return HigherPrecisionTypeForComplexArithmetic(ElementType, | ||
| IsDivOpCode); | ||
| if (ElementType.UseExcessPrecision(CGF.getContext())) | ||
| return CGF.getContext().getComplexType(CGF.getContext().FloatTy); | ||
| } | ||
|
|
@@ -296,11 +333,12 @@ class ComplexExprEmitter | |
|
|
||
| #define HANDLEBINOP(OP) \ | ||
| ComplexPairTy VisitBin##OP(const BinaryOperator *E) { \ | ||
| QualType promotionTy = getPromotionType( \ | ||
| E->getType(), \ | ||
| (E->getOpcode() == BinaryOperatorKind::BO_Div) ? true : false); \ | ||
| ComplexPairTy result = EmitBin##OP(EmitBinOps(E, promotionTy)); \ | ||
| if (!promotionTy.isNull()) \ | ||
| result = CGF.EmitUnPromotedValue(result, E->getType()); \ | ||
| return result; \ | ||
| } | ||
|
|
||
|
|
@@ -790,8 +828,10 @@ ComplexPairTy ComplexExprEmitter::EmitBinMul(const BinOpInfo &Op) { | |
| ResR = Builder.CreateFSub(AC, BD, "mul_r"); | ||
| ResI = Builder.CreateFAdd(AD, BC, "mul_i"); | ||
|
|
||
| if (Op.FPFeatures.getComplexRange() == LangOptions::CX_Basic || | ||
| Op.FPFeatures.getComplexRange() == LangOptions::CX_Improved || | ||
| Op.FPFeatures.getComplexRange() == LangOptions::CX_Promoted || | ||
| CGF.getLangOpts().NoHonorInfs || CGF.getLangOpts().NoHonorNaNs) | ||
|
There was a problem hiding this comment. I'm not sure NoHonorInfs or NoHonorNaNs should be checked here. Given that we have explicit control for complex arithmetic behavior, maybe that should take precedence. That seems to be the way gcc handles it: https://godbolt.org/z/1oGo7jznz There was a problem hiding this comment. This code is in There was a problem hiding this comment. Oh, sorry, for some reason I thought this was invoking the special handling for division. I think the for honor NaNs and infinities still isn't necessary because we'll emit the comparison with the fast-math flags set and the backend will optimize it away, which is what happens today: https://godbolt.org/z/e8EnKodj6 |
||
| return ComplexPairTy(ResR, ResI); | ||
|
|
||
| // Emit the test for the real part becoming NaN and create a branch to | ||
|
|
@@ -982,13 +1022,18 @@ ComplexPairTy ComplexExprEmitter::EmitBinDiv(const BinOpInfo &Op) { | |
| llvm::Value *OrigLHSi = LHSi; | ||
| if (!LHSi) | ||
| LHSi = llvm::Constant::getNullValue(RHSi->getType()); | ||
| QualType ComplexElementTy = Op.Ty->castAs<ComplexType>()->getElementType(); | ||
| const BuiltinType *BT = ComplexElementTy->getAs<BuiltinType>(); | ||
| if (Op.FPFeatures.getComplexRange() == LangOptions::CX_Improved || | ||
| (Op.FPFeatures.getComplexRange() == LangOptions::CX_Promoted && | ||
| BT->getKind() == BuiltinType::Kind::LongDouble)) | ||
|
There was a problem hiding this comment. This isn't going to do the right thing for |
||
| return EmitRangeReductionDiv(LHSr, LHSi, RHSr, RHSi); | ||
| else if (Op.FPFeatures.getComplexRange() == LangOptions::CX_Basic || | ||
| Op.FPFeatures.getComplexRange() == LangOptions::CX_Promoted) | ||
| return EmitAlgebraicDiv(LHSr, LHSi, RHSr, RHSi); | ||
| else if (!CGF.getLangOpts().FastMath || | ||
|
There was a problem hiding this comment. I think we should remove the fast-math check here. The driver handling of fast-math sets the complex arithmetic option. This check has always been problematic because disabling just one component of fast-math (such as enabling signed zeros) causes this to be false. |
||
| // '-ffast-math' is used in the command line but followed by an | ||
| // '-fno-cx-limited-range' or '-fcomplex-arithmetic=full'. | ||
| Op.FPFeatures.getComplexRange() == LangOptions::CX_Full) { | ||
| LHSi = OrigLHSi; | ||
| // If we have a complex operand on the RHS and FastMath is not allowed, we | ||
|
|
||
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.
This documentation doesn't make it clear what happens if there is no higher-precision datatype available and you use
promotedformat. And I'm somewhat uncomfortable with the idea that usingpromotedkeeps you from being able to choose what happens in that case.(In general,
promotedis scary to me for anything larger than afloatbecauselong doubleis just such a cursed type and I'm not sure it's a good idea to convertdoublecomputations tolong double).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.
The intention here was that if the target doesn't support a higher precision type, we will do what we would have done with "improved". Some targets don't even support a 64-bit floating-point type, so the way we apply this needs to be generalized. Should we issue a warning if the user specifies "promoted" but we can't promote?
Zahira and I talked about this offline, and my suggestion was that if LongDoubleSize is greater than DoubleSize, we can promote double to long double, but if it isn't we will use the Smith algorithm (i.e. "improved"). Windows on x86-64 is the really ugly case here, because the target hardware supports an 80-bit floating-point type, but by default the operating system configures the x87 layer to perform calculations as if it were a 64-bit type.
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.
My understanding of the x87 precision control field is that it only affects the number of used bits in the significand, but still retains the full exponent range, so even with PC set to 53-bit precision, you would still get sufficient range to avoid overflow in a complex division, albeit the result might be rounded slightly differently.