[PATCH] D157596: [clang][Interp] Handle mixed floating/integral compound assign operators
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGadb1fb40e84d: [clang][Interp] Handle mixed floating/integral
compound assign operators (authored by tbaeder).
Changed prior to commit:
https://reviews.llvm.org/D157596?vs=551434&id=555839#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157596/new/
https://reviews.llvm.org/D157596
Files:
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeExprGen.h
clang/lib/AST/Interp/PrimType.h
clang/test/AST/Interp/floats.cpp
Index: clang/test/AST/Interp/floats.cpp
===
--- clang/test/AST/Interp/floats.cpp
+++ clang/test/AST/Interp/floats.cpp
@@ -102,6 +102,38 @@
return a[1];
}
static_assert(ff() == 3, "");
+
+ constexpr float intPlusDouble() {
+ int a = 0;
+ a += 2.0;
+
+ return a;
+ }
+ static_assert(intPlusDouble() == 2, "");
+
+ constexpr double doublePlusInt() {
+ double a = 0.0;
+ a += 2;
+
+ return a;
+ }
+ static_assert(doublePlusInt() == 2, "");
+
+ constexpr float boolPlusDouble() {
+ bool a = 0;
+ a += 1.0;
+
+ return a;
+ }
+ static_assert(boolPlusDouble(), "");
+
+ constexpr bool doublePlusbool() {
+ double a = 0.0;
+ a += true;
+
+ return a;
+ }
+ static_assert(doublePlusbool() == 1.0, "");
}
namespace unary {
Index: clang/lib/AST/Interp/PrimType.h
===
--- clang/lib/AST/Interp/PrimType.h
+++ clang/lib/AST/Interp/PrimType.h
@@ -56,7 +56,7 @@
return OS;
}
-constexpr bool isIntegralType(PrimType T) { return T <= PT_Uint64; }
+constexpr bool isIntegralType(PrimType T) { return T <= PT_Bool; }
/// Mapping from primitive types to their representation.
template struct PrimConv;
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -265,6 +265,7 @@
return FPO.getRoundingMode();
}
+ bool emitPrimCast(PrimType FromT, PrimType ToT, QualType ToQT, const Expr *E);
bool emitRecordDestruction(const Descriptor *Desc);
bool emitDerivedToBaseCasts(const RecordType *DerivedType,
const RecordType *BaseType, const Expr *E);
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -878,19 +878,22 @@
template
bool ByteCodeExprGen::VisitFloatCompoundAssignOperator(
const CompoundAssignOperator *E) {
- assert(E->getType()->isFloatingType());
const Expr *LHS = E->getLHS();
const Expr *RHS = E->getRHS();
- llvm::RoundingMode RM = getRoundingMode(E);
+ QualType LHSType = LHS->getType();
QualType LHSComputationType = E->getComputationLHSType();
QualType ResultType = E->getComputationResultType();
std::optional LT = classify(LHSComputationType);
std::optional RT = classify(ResultType);
+ assert(ResultType->isFloatingType());
+
if (!LT || !RT)
return false;
+ PrimType LHST = classifyPrim(LHSType);
+
// C++17 onwards require that we evaluate the RHS first.
// Compute RHS and save it in a temporary variable so we can
// load it again later.
@@ -904,21 +907,19 @@
// First, visit LHS.
if (!visit(LHS))
return false;
- if (!this->emitLoad(*LT, E))
+ if (!this->emitLoad(LHST, E))
return false;
// If necessary, convert LHS to its computation type.
- if (LHS->getType() != LHSComputationType) {
-const auto *TargetSemantics = &Ctx.getFloatSemantics(LHSComputationType);
-
-if (!this->emitCastFP(TargetSemantics, RM, E))
- return false;
- }
+ if (!this->emitPrimCast(LHST, classifyPrim(LHSComputationType),
+ LHSComputationType, E))
+return false;
// Now load RHS.
if (!this->emitGetLocal(*RT, TempOffset, E))
return false;
+ llvm::RoundingMode RM = getRoundingMode(E);
switch (E->getOpcode()) {
case BO_AddAssign:
if (!this->emitAddf(RM, E))
@@ -940,17 +941,12 @@
return false;
}
- // If necessary, convert result to LHS's type.
- if (LHS->getType() != ResultType) {
-const auto *TargetSemantics = &Ctx.getFloatSemantics(LHS->getType());
-
-if (!this->emitCastFP(TargetSemantics, RM, E))
- return false;
- }
+ if (!this->emitPrimCast(classifyPrim(ResultType), LHST, LHS->getType(), E))
+return false;
if (DiscardResult)
-return this->emitStorePop(*LT, E);
- return this->emitStore(*LT, E);
+return this->emitStorePop(LHST, E);
+ return this->emitStore(LHST, E);
}
template
@@ -992,14 +988,6 @@
bool ByteCodeExprGen::VisitCompoundAssignOperator(
const CompoundAssignOperator *E) {
- // Handle floating point op
[PATCH] D157596: [clang][Interp] Handle mixed floating/integral compound assign operators
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM with a testing nit.
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:2659-2664
+ if (FromT == PT_Float) {
+// Floating to floating.
+if (ToT == PT_Float) {
+ const llvm::fltSemantics *ToSem = &Ctx.getFloatSemantics(ToQT);
+ return this->emitCastFP(ToSem, getRoundingMode(E), E);
+}
tbaeder wrote:
> aaron.ballman wrote:
> > Should we be early returning if we're casting from float->float like we do
> > for int->int?
> Heh, I was thinking the exact same thing when writing this, but
>
> a) That means we also have to pass a `QualTyp FromQT`
> b) I don't think we'd ever run into this problem since the AST would have to
> contain such a cast from/to the same floating type.
Okay, that seems reasonable to me. We can always revisit later if we find it on
a hot path for some reason.
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:2667
+// Float to integral.
+if (isIntegralType(ToT) || ToT == PT_Bool)
+ return this->emitCastFloatingIntegral(ToT, E);
tbaeder wrote:
> aaron.ballman wrote:
> > It weirds me out that `isIntegralType()` returns false for a boolean type;
> > that is an integral type as well:
> > http://eel.is/c++draft/basic.types#basic.fundamental-11
> I don't think there's a good reason for it to return `false` for bools;
> before this patch, the function is only used for an assertion in `Neg()`.
Sounds like a good NFC change to make as a follow-up.
Comment at: clang/test/AST/Interp/floats.cpp:106
+
+ constexpr float intPlusDouble() {
+ int a = 0;
tbaeder wrote:
> aaron.ballman wrote:
> > Is it intentional that this is returning a float but then comparing below
> > as an integer?
> Kinda? What we compare against doesn't really matter for the compound
> assignment in the function, does it?
Heh, that's why I was surprised -- it seems like odd type mismatches that are
unrelated to the patch. If that's not what's being tested, might as well match
the types up better so the test coverage is more clear.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157596/new/
https://reviews.llvm.org/D157596
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D157596: [clang][Interp] Handle mixed floating/integral compound assign operators
tbaeder updated this revision to Diff 551434.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157596/new/
https://reviews.llvm.org/D157596
Files:
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeExprGen.h
clang/lib/AST/Interp/PrimType.h
clang/test/AST/Interp/floats.cpp
Index: clang/test/AST/Interp/floats.cpp
===
--- clang/test/AST/Interp/floats.cpp
+++ clang/test/AST/Interp/floats.cpp
@@ -102,6 +102,38 @@
return a[1];
}
static_assert(ff() == 3, "");
+
+ constexpr float intPlusDouble() {
+ int a = 0;
+ a += 2.0;
+
+ return a;
+ }
+ static_assert(intPlusDouble() == 2, "");
+
+ constexpr double doublePlusInt() {
+ double a = 0.0;
+ a += 2;
+
+ return a;
+ }
+ static_assert(doublePlusInt() == 2, "");
+
+ constexpr float boolPlusDouble() {
+ bool a = 0;
+ a += 1.0;
+
+ return a;
+ }
+ static_assert(boolPlusDouble(), "");
+
+ constexpr bool doublePlusbool() {
+ double a = 0.0;
+ a += true;
+
+ return a;
+ }
+ static_assert(doublePlusbool() == 1.0, "");
}
namespace unary {
Index: clang/lib/AST/Interp/PrimType.h
===
--- clang/lib/AST/Interp/PrimType.h
+++ clang/lib/AST/Interp/PrimType.h
@@ -56,7 +56,7 @@
return OS;
}
-constexpr bool isIntegralType(PrimType T) { return T <= PT_Uint64; }
+constexpr bool isIntegralType(PrimType T) { return T <= PT_Bool; }
/// Mapping from primitive types to their representation.
template struct PrimConv;
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -276,6 +276,7 @@
return FPO.getRoundingMode();
}
+ bool emitPrimCast(PrimType FromT, PrimType ToT, QualType ToQT, const Expr *E);
bool emitRecordDestruction(const Descriptor *Desc);
bool emitDerivedToBaseCasts(const RecordType *DerivedType,
const RecordType *BaseType, const Expr *E);
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -986,19 +986,22 @@
template
bool ByteCodeExprGen::VisitFloatCompoundAssignOperator(
const CompoundAssignOperator *E) {
- assert(E->getType()->isFloatingType());
const Expr *LHS = E->getLHS();
const Expr *RHS = E->getRHS();
- llvm::RoundingMode RM = getRoundingMode(E);
+ QualType LHSType = LHS->getType();
QualType LHSComputationType = E->getComputationLHSType();
QualType ResultType = E->getComputationResultType();
std::optional LT = classify(LHSComputationType);
std::optional RT = classify(ResultType);
+ assert(ResultType->isFloatingType());
+
if (!LT || !RT)
return false;
+ PrimType LHST = classifyPrim(LHSType);
+
// C++17 onwards require that we evaluate the RHS first.
// Compute RHS and save it in a temporary variable so we can
// load it again later.
@@ -1012,21 +1015,19 @@
// First, visit LHS.
if (!visit(LHS))
return false;
- if (!this->emitLoad(*LT, E))
+ if (!this->emitLoad(LHST, E))
return false;
// If necessary, convert LHS to its computation type.
- if (LHS->getType() != LHSComputationType) {
-const auto *TargetSemantics = &Ctx.getFloatSemantics(LHSComputationType);
-
-if (!this->emitCastFP(TargetSemantics, RM, E))
- return false;
- }
+ if (!this->emitPrimCast(LHST, classifyPrim(LHSComputationType),
+ LHSComputationType, E))
+return false;
// Now load RHS.
if (!this->emitGetLocal(*RT, TempOffset, E))
return false;
+ llvm::RoundingMode RM = getRoundingMode(E);
switch (E->getOpcode()) {
case BO_AddAssign:
if (!this->emitAddf(RM, E))
@@ -1048,17 +1049,12 @@
return false;
}
- // If necessary, convert result to LHS's type.
- if (LHS->getType() != ResultType) {
-const auto *TargetSemantics = &Ctx.getFloatSemantics(LHS->getType());
-
-if (!this->emitCastFP(TargetSemantics, RM, E))
- return false;
- }
+ if (!this->emitPrimCast(classifyPrim(ResultType), LHST, LHS->getType(), E))
+return false;
if (DiscardResult)
-return this->emitStorePop(*LT, E);
- return this->emitStore(*LT, E);
+return this->emitStorePop(LHST, E);
+ return this->emitStore(LHST, E);
}
template
@@ -1100,14 +1096,6 @@
bool ByteCodeExprGen::VisitCompoundAssignOperator(
const CompoundAssignOperator *E) {
- // Handle floating point operations separately here, since they
- // require special care.
- if (E->getType()->isFloatingType())
-return VisitFloatCompoundAssignOperator(E);
-
- if (E->getType()->isPointerType())
-return VisitPointerCompoundAssignOperator(E);
-
const Expr *LHS = E->getLHS();
const Expr *RHS = E->getRHS();
std::optional L
[PATCH] D157596: [clang][Interp] Handle mixed floating/integral compound assign operators
tbaeder marked an inline comment as done.
tbaeder added inline comments.
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:2659-2664
+ if (FromT == PT_Float) {
+// Floating to floating.
+if (ToT == PT_Float) {
+ const llvm::fltSemantics *ToSem = &Ctx.getFloatSemantics(ToQT);
+ return this->emitCastFP(ToSem, getRoundingMode(E), E);
+}
aaron.ballman wrote:
> Should we be early returning if we're casting from float->float like we do
> for int->int?
Heh, I was thinking the exact same thing when writing this, but
a) That means we also have to pass a `QualTyp FromQT`
b) I don't think we'd ever run into this problem since the AST would have to
contain such a cast from/to the same floating type.
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:2667
+// Float to integral.
+if (isIntegralType(ToT) || ToT == PT_Bool)
+ return this->emitCastFloatingIntegral(ToT, E);
aaron.ballman wrote:
> It weirds me out that `isIntegralType()` returns false for a boolean type;
> that is an integral type as well:
> http://eel.is/c++draft/basic.types#basic.fundamental-11
I don't think there's a good reason for it to return `false` for bools; before
this patch, the function is only used for an assertion in `Neg()`.
Comment at: clang/test/AST/Interp/floats.cpp:106
+
+ constexpr float intPlusDouble() {
+ int a = 0;
aaron.ballman wrote:
> Is it intentional that this is returning a float but then comparing below as
> an integer?
Kinda? What we compare against doesn't really matter for the compound
assignment in the function, does it?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157596/new/
https://reviews.llvm.org/D157596
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D157596: [clang][Interp] Handle mixed floating/integral compound assign operators
aaron.ballman added inline comments.
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:2654
+/// Emit casts from a PrimType to another PrimType
+template
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:2659-2664
+ if (FromT == PT_Float) {
+// Floating to floating.
+if (ToT == PT_Float) {
+ const llvm::fltSemantics *ToSem = &Ctx.getFloatSemantics(ToQT);
+ return this->emitCastFP(ToSem, getRoundingMode(E), E);
+}
Should we be early returning if we're casting from float->float like we do for
int->int?
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:2667
+// Float to integral.
+if (isIntegralType(ToT) || ToT == PT_Bool)
+ return this->emitCastFloatingIntegral(ToT, E);
It weirds me out that `isIntegralType()` returns false for a boolean type; that
is an integral type as well:
http://eel.is/c++draft/basic.types#basic.fundamental-11
Comment at: clang/test/AST/Interp/floats.cpp:106
+
+ constexpr float intPlusDouble() {
+ int a = 0;
Is it intentional that this is returning a float but then comparing below as an
integer?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157596/new/
https://reviews.llvm.org/D157596
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D157596: [clang][Interp] Handle mixed floating/integral compound assign operators
tbaeder updated this revision to Diff 551132.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157596/new/
https://reviews.llvm.org/D157596
Files:
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeExprGen.h
clang/test/AST/Interp/floats.cpp
Index: clang/test/AST/Interp/floats.cpp
===
--- clang/test/AST/Interp/floats.cpp
+++ clang/test/AST/Interp/floats.cpp
@@ -102,6 +102,38 @@
return a[1];
}
static_assert(ff() == 3, "");
+
+ constexpr float intPlusDouble() {
+ int a = 0;
+ a += 2.0;
+
+ return a;
+ }
+ static_assert(intPlusDouble() == 2, "");
+
+ constexpr double doublePlusInt() {
+ double a = 0.0;
+ a += 2;
+
+ return a;
+ }
+ static_assert(doublePlusInt() == 2, "");
+
+ constexpr float boolPlusDouble() {
+ bool a = 0;
+ a += 1.0;
+
+ return a;
+ }
+ static_assert(boolPlusDouble(), "");
+
+ constexpr bool doublePlusbool() {
+ double a = 0.0;
+ a += true;
+
+ return a;
+ }
+ static_assert(doublePlusbool() == 1.0, "");
}
namespace unary {
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -276,6 +276,7 @@
return FPO.getRoundingMode();
}
+ bool emitPrimCast(PrimType FromT, PrimType ToT, QualType ToQT, const Expr *E);
bool emitRecordDestruction(const Descriptor *Desc);
bool emitDerivedToBaseCasts(const RecordType *DerivedType,
const RecordType *BaseType, const Expr *E);
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -986,19 +986,22 @@
template
bool ByteCodeExprGen::VisitFloatCompoundAssignOperator(
const CompoundAssignOperator *E) {
- assert(E->getType()->isFloatingType());
const Expr *LHS = E->getLHS();
const Expr *RHS = E->getRHS();
- llvm::RoundingMode RM = getRoundingMode(E);
+ QualType LHSType = LHS->getType();
QualType LHSComputationType = E->getComputationLHSType();
QualType ResultType = E->getComputationResultType();
std::optional LT = classify(LHSComputationType);
std::optional RT = classify(ResultType);
+ assert(ResultType->isFloatingType());
+
if (!LT || !RT)
return false;
+ PrimType LHST = classifyPrim(LHSType);
+
// C++17 onwards require that we evaluate the RHS first.
// Compute RHS and save it in a temporary variable so we can
// load it again later.
@@ -1012,21 +1015,19 @@
// First, visit LHS.
if (!visit(LHS))
return false;
- if (!this->emitLoad(*LT, E))
+ if (!this->emitLoad(LHST, E))
return false;
// If necessary, convert LHS to its computation type.
- if (LHS->getType() != LHSComputationType) {
-const auto *TargetSemantics = &Ctx.getFloatSemantics(LHSComputationType);
-
-if (!this->emitCastFP(TargetSemantics, RM, E))
- return false;
- }
+ if (!this->emitPrimCast(LHST, classifyPrim(LHSComputationType),
+ LHSComputationType, E))
+return false;
// Now load RHS.
if (!this->emitGetLocal(*RT, TempOffset, E))
return false;
+ llvm::RoundingMode RM = getRoundingMode(E);
switch (E->getOpcode()) {
case BO_AddAssign:
if (!this->emitAddf(RM, E))
@@ -1048,17 +1049,12 @@
return false;
}
- // If necessary, convert result to LHS's type.
- if (LHS->getType() != ResultType) {
-const auto *TargetSemantics = &Ctx.getFloatSemantics(LHS->getType());
-
-if (!this->emitCastFP(TargetSemantics, RM, E))
- return false;
- }
+ if (!this->emitPrimCast(classifyPrim(ResultType), LHST, LHS->getType(), E))
+return false;
if (DiscardResult)
-return this->emitStorePop(*LT, E);
- return this->emitStore(*LT, E);
+return this->emitStorePop(LHST, E);
+ return this->emitStore(LHST, E);
}
template
@@ -1100,14 +1096,6 @@
bool ByteCodeExprGen::VisitCompoundAssignOperator(
const CompoundAssignOperator *E) {
- // Handle floating point operations separately here, since they
- // require special care.
- if (E->getType()->isFloatingType())
-return VisitFloatCompoundAssignOperator(E);
-
- if (E->getType()->isPointerType())
-return VisitPointerCompoundAssignOperator(E);
-
const Expr *LHS = E->getLHS();
const Expr *RHS = E->getRHS();
std::optional LHSComputationT =
@@ -1120,6 +1108,15 @@
if (!LT || !RT || !RHST || !ResultT || !LHSComputationT)
return false;
+ // Handle floating point operations separately here, since they
+ // require special care.
+
+ if (ResultT == PT_Float || RT == PT_Float)
+return VisitFloatCompoundAssignOperator(E);
+
+ if (E->getType()->isPointerType())
+return VisitPointerCompoundAssignOperator(E);
+
assert(!E->getType()->isPointerType() && "Handled above");
assert
[PATCH] D157596: [clang][Interp] Handle mixed floating/integral compound assign operators
cor3ntin added inline comments.
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:2676
+
+if (ToT == PT_Float) {
+ const llvm::fltSemantics *ToSem = &Ctx.getFloatSemantics(ToQT);
Might as well leave a comment here too, for symmetry
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157596/new/
https://reviews.llvm.org/D157596
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D157596: [clang][Interp] Handle mixed floating/integral compound assign operators
tbaeder added a comment. Ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157596/new/ https://reviews.llvm.org/D157596 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D157596: [clang][Interp] Handle mixed floating/integral compound assign operators
tbaeder updated this revision to Diff 548985.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157596/new/
https://reviews.llvm.org/D157596
Files:
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeExprGen.h
clang/test/AST/Interp/floats.cpp
Index: clang/test/AST/Interp/floats.cpp
===
--- clang/test/AST/Interp/floats.cpp
+++ clang/test/AST/Interp/floats.cpp
@@ -102,6 +102,38 @@
return a[1];
}
static_assert(ff() == 3, "");
+
+ constexpr float intPlusDouble() {
+ int a = 0;
+ a += 2.0;
+
+ return a;
+ }
+ static_assert(intPlusDouble() == 2, "");
+
+ constexpr double doublePlusInt() {
+ double a = 0.0;
+ a += 2;
+
+ return a;
+ }
+ static_assert(doublePlusInt() == 2, "");
+
+ constexpr float boolPlusDouble() {
+ bool a = 0;
+ a += 1.0;
+
+ return a;
+ }
+ static_assert(boolPlusDouble(), "");
+
+ constexpr bool doublePlusbool() {
+ double a = 0.0;
+ a += true;
+
+ return a;
+ }
+ static_assert(doublePlusbool() == 1.0, "");
}
namespace unary {
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -276,6 +276,7 @@
return FPO.getRoundingMode();
}
+ bool emitPrimCast(PrimType FromT, PrimType ToT, QualType ToQT, const Expr *E);
bool emitRecordDestruction(const Descriptor *Desc);
bool emitDerivedToBaseCasts(const RecordType *DerivedType,
const RecordType *BaseType, const Expr *E);
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -986,19 +986,22 @@
template
bool ByteCodeExprGen::VisitFloatCompoundAssignOperator(
const CompoundAssignOperator *E) {
- assert(E->getType()->isFloatingType());
const Expr *LHS = E->getLHS();
const Expr *RHS = E->getRHS();
- llvm::RoundingMode RM = getRoundingMode(E);
+ QualType LHSType = LHS->getType();
QualType LHSComputationType = E->getComputationLHSType();
QualType ResultType = E->getComputationResultType();
std::optional LT = classify(LHSComputationType);
std::optional RT = classify(ResultType);
+ assert(ResultType->isFloatingType());
+
if (!LT || !RT)
return false;
+ PrimType LHST = classifyPrim(LHSType);
+
// C++17 onwards require that we evaluate the RHS first.
// Compute RHS and save it in a temporary variable so we can
// load it again later.
@@ -1012,21 +1015,19 @@
// First, visit LHS.
if (!visit(LHS))
return false;
- if (!this->emitLoad(*LT, E))
+ if (!this->emitLoad(LHST, E))
return false;
// If necessary, convert LHS to its computation type.
- if (LHS->getType() != LHSComputationType) {
-const auto *TargetSemantics = &Ctx.getFloatSemantics(LHSComputationType);
-
-if (!this->emitCastFP(TargetSemantics, RM, E))
- return false;
- }
+ if (!this->emitPrimCast(LHST, classifyPrim(LHSComputationType),
+ LHSComputationType, E))
+return false;
// Now load RHS.
if (!this->emitGetLocal(*RT, TempOffset, E))
return false;
+ llvm::RoundingMode RM = getRoundingMode(E);
switch (E->getOpcode()) {
case BO_AddAssign:
if (!this->emitAddf(RM, E))
@@ -1048,17 +1049,12 @@
return false;
}
- // If necessary, convert result to LHS's type.
- if (LHS->getType() != ResultType) {
-const auto *TargetSemantics = &Ctx.getFloatSemantics(LHS->getType());
-
-if (!this->emitCastFP(TargetSemantics, RM, E))
- return false;
- }
+ if (!this->emitPrimCast(classifyPrim(ResultType), LHST, LHS->getType(), E))
+return false;
if (DiscardResult)
-return this->emitStorePop(*LT, E);
- return this->emitStore(*LT, E);
+return this->emitStorePop(LHST, E);
+ return this->emitStore(LHST, E);
}
template
@@ -1100,14 +1096,6 @@
bool ByteCodeExprGen::VisitCompoundAssignOperator(
const CompoundAssignOperator *E) {
- // Handle floating point operations separately here, since they
- // require special care.
- if (E->getType()->isFloatingType())
-return VisitFloatCompoundAssignOperator(E);
-
- if (E->getType()->isPointerType())
-return VisitPointerCompoundAssignOperator(E);
-
const Expr *LHS = E->getLHS();
const Expr *RHS = E->getRHS();
std::optional LHSComputationT =
@@ -1120,6 +1108,15 @@
if (!LT || !RT || !RHST || !ResultT || !LHSComputationT)
return false;
+ // Handle floating point operations separately here, since they
+ // require special care.
+
+ if (ResultT == PT_Float || RT == PT_Float)
+return VisitFloatCompoundAssignOperator(E);
+
+ if (E->getType()->isPointerType())
+return VisitPointerCompoundAssignOperator(E);
+
assert(!E->getType()->isPointerType() && "Handled above");
assert
[PATCH] D157596: [clang][Interp] Handle mixed floating/integral compound assign operators
tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, shafik, cor3ntin.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added subscribers: cfe-commits, wangpc.
Herald added a project: clang.
Add a new `emitPrimCast` helper function and use that when evaluating floating
compound operators.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D157596
Files:
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeExprGen.h
clang/test/AST/Interp/floats.cpp
Index: clang/test/AST/Interp/floats.cpp
===
--- clang/test/AST/Interp/floats.cpp
+++ clang/test/AST/Interp/floats.cpp
@@ -102,6 +102,22 @@
return a[1];
}
static_assert(ff() == 3, "");
+
+ constexpr float intPlusDouble() {
+ int a = 0;
+ a += 2.0;
+
+ return a;
+ }
+ static_assert(intPlusDouble() == 2, "");
+
+ constexpr float doublePlusInt() {
+ double a = 0.0;
+ a += 2;
+
+ return a;
+ }
+ static_assert(doublePlusInt() == 2, "");
}
namespace unary {
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -276,6 +276,7 @@
return FPO.getRoundingMode();
}
+ bool emitPrimCast(PrimType FromT, PrimType ToT, QualType ToQT, const Expr *E);
bool emitRecordDestruction(const Descriptor *Desc);
bool emitDerivedToBaseCasts(const RecordType *DerivedType,
const RecordType *BaseType, const Expr *E);
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -986,19 +986,22 @@
template
bool ByteCodeExprGen::VisitFloatCompoundAssignOperator(
const CompoundAssignOperator *E) {
- assert(E->getType()->isFloatingType());
const Expr *LHS = E->getLHS();
const Expr *RHS = E->getRHS();
- llvm::RoundingMode RM = getRoundingMode(E);
+ QualType LHSType = LHS->getType();
QualType LHSComputationType = E->getComputationLHSType();
QualType ResultType = E->getComputationResultType();
std::optional LT = classify(LHSComputationType);
std::optional RT = classify(ResultType);
+ assert(ResultType->isFloatingType());
+
if (!LT || !RT)
return false;
+ PrimType LHST = classifyPrim(LHSType);
+
// C++17 onwards require that we evaluate the RHS first.
// Compute RHS and save it in a temporary variable so we can
// load it again later.
@@ -1012,21 +1015,19 @@
// First, visit LHS.
if (!visit(LHS))
return false;
- if (!this->emitLoad(*LT, E))
+ if (!this->emitLoad(LHST, E))
return false;
// If necessary, convert LHS to its computation type.
- if (LHS->getType() != LHSComputationType) {
-const auto *TargetSemantics = &Ctx.getFloatSemantics(LHSComputationType);
-
-if (!this->emitCastFP(TargetSemantics, RM, E))
- return false;
- }
+ if (!this->emitPrimCast(LHST, classifyPrim(LHSComputationType),
+ LHSComputationType, E))
+return false;
// Now load RHS.
if (!this->emitGetLocal(*RT, TempOffset, E))
return false;
+ llvm::RoundingMode RM = getRoundingMode(E);
switch (E->getOpcode()) {
case BO_AddAssign:
if (!this->emitAddf(RM, E))
@@ -1048,17 +1049,12 @@
return false;
}
- // If necessary, convert result to LHS's type.
- if (LHS->getType() != ResultType) {
-const auto *TargetSemantics = &Ctx.getFloatSemantics(LHS->getType());
-
-if (!this->emitCastFP(TargetSemantics, RM, E))
- return false;
- }
+ if (!this->emitPrimCast(classifyPrim(ResultType), LHST, LHS->getType(), E))
+return false;
if (DiscardResult)
-return this->emitStorePop(*LT, E);
- return this->emitStore(*LT, E);
+return this->emitStorePop(LHST, E);
+ return this->emitStore(LHST, E);
}
template
@@ -1100,14 +1096,6 @@
bool ByteCodeExprGen::VisitCompoundAssignOperator(
const CompoundAssignOperator *E) {
- // Handle floating point operations separately here, since they
- // require special care.
- if (E->getType()->isFloatingType())
-return VisitFloatCompoundAssignOperator(E);
-
- if (E->getType()->isPointerType())
-return VisitPointerCompoundAssignOperator(E);
-
const Expr *LHS = E->getLHS();
const Expr *RHS = E->getRHS();
std::optional LHSComputationT =
@@ -1120,6 +1108,15 @@
if (!LT || !RT || !RHST || !ResultT || !LHSComputationT)
return false;
+ // Handle floating point operations separately here, since they
+ // require special care.
+
+ if (ResultT == PT_Float || RT == PT_Float)
+return VisitFloatCompoundAssignOperator(E);
+
+ if (E->getType()->isPointerType())
+return VisitPointerCompoundAssignOperator(E);
+
assert(!E->getType()->isPointerType() && "Handled above");
a
