This only handles floats for now, so 32 bit floating point values.
There are a few things I'm uncertain about, like how to implement Floating<>::truncate() or whether or not Float32 should be part of AluTypeClass.
Differential D134859
[clang][Interp] Implement basic support for floating point values tbaeder on Sep 29 2022, 1:10 AM. Authored by
Details This only handles floats for now, so 32 bit floating point values. There are a few things I'm uncertain about, like how to implement Floating<>::truncate() or whether or not Float32 should be part of AluTypeClass.
Diff Detail
Event Timeline
Comment Actions I've not given this a full review yet, but I do have some comments on a partial review. Mostly, I'm concerned we're going to have different semantics when evaluating than we'd get if the evaluation happened at runtime because we're using the host floating-point environment and not the target. I'm also a bit concerned the design won't extend very easily to long double support. We don't have to solve all of this with the initial offering, but I think we want to be sure we're building the float support on a stable base. Adding @jcranmer-intel for another set of eyes on this code from someone who knows many of the most terrifying parts of floating-point support.
Comment Actions Some extra constexpr tests that may be interesting:
Some of these tests relate to things that you haven't implemented yet--and that's fine--but it's worth keeping in mind for future patches.
Comment Actions Another try, this time without any underlying host type. There are a few things I'm uncertain about. The tests are obviously not enough, especially now that I had to add another CastFP opcode to cast floats between different semantics. The general approach seems much saner than before, but I'm sure you'll find enough problem with this one as well :) BTW, I feel like there are a lot of comments on this review now, so if you'd like me to split the new version of the patch out into another ticket, I can do that of course.
Comment Actions Another (and even longer) version. Introduce special opcodes for floating point operations and pass the rounding mode to them. Also create special int->float and float->int casts so we can handle that properly. This makes clang/tests/SemaCXX/rouding-math.cpp work. I've only enabled one of the two RUN lines though. The other one fails because we don't handle CXXNewExprs yet.
Comment Actions Moved the tests to their own file and moved the isConstantContext() changes to InterpState to their own NFC commit.
Comment Actions Added a const-fpfeatures.cpp test and added semantics converstion to ::add as an example.
Comment Actions Return opStatus from convertToInteger as well, and add a test case to floats.cpp that tests float-to-integer conversion.
Comment Actions FYI, I noticed the way the floating values are serialized doesn't work if the APFloat heap-allocated anything; those values aren't preserved through (de)serialization of course. Reproducer: constexpr double foo() { return __LDBL_MIN__; } Comment Actions The return statement returns a value of type long double while the function returns double. If long double is wider than double, truncation occurs, may be this is the reason? Comment Actions It also happens when the function returns a long double. I tracked the problem down to the way we emit byte code into a std::vector. It's simply reintepret_cast'ed to const char *, so that won't work for APFloat. This change seems to fix the problem: diff --git a/clang/lib/AST/Interp/ByteCodeEmitter.cpp b/clang/lib/AST/Interp/ByteCodeEmitter.cpp index ff2136d34872..61e9662013c3 100644 --- a/clang/lib/AST/Interp/ByteCodeEmitter.cpp +++ b/clang/lib/AST/Interp/ByteCodeEmitter.cpp @@ -163,8 +163,15 @@ static void emit(Program &P, std::vector<char> &Code, const T &Val, } if constexpr (!std::is_pointer_v<T>) { - const char *Data = reinterpret_cast<const char *>(&Val); - Code.insert(Code.end(), Data, Data + Size); + if constexpr (std::is_trivially_copyable_v<T>) { + const char *Data = reinterpret_cast<const char *>(&Val); + Code.insert(Code.end(), Data, Data + Size); + } else { + // Construct the value directly into our storage vector. + size_t ValPos = Code.size(); + Code.resize(Code.size() + Size); + new (Code.data() + ValPos) T(Val); + } } else { uint32_t ID = P.getOrCreateNativePointer(Val); const char *Data = reinterpret_cast<const char *>(&ID); Comment Actions Remove some questionable (but unused) Floating API that didn't take floating-point semantics into account.
Comment Actions Remove conversion in Floating::add() and tests that relied on it. Compound assignment operators can be properly implemented later.
|
Is there any reason why operator double exists, but operator float does not?