ebevhan added a comment. In https://reviews.llvm.org/D53738#1283983, @rjmccall wrote:
> In https://reviews.llvm.org/D53738#1283861, @ebevhan wrote: > > > In https://reviews.llvm.org/D53738#1283459, @rjmccall wrote: > > > > > Well, it could be passed around through most code as some sort of > > > abstractly-represented intermediate "type" which could be either a > > > QualType or a fixed-point semantics. > > > > > > Sounds to me like you're describing a new `Type` that can contain an > > arbitrary fixed-point semantic :) > > > The fact that it doesn't exist in the modeled type system is important. > > The arbitrary-type overflow intrinsics have this same problem, and we did not > try to solve it by creating a new type class for arbitrary-precision integers > and promoting the arguments. Okay, I had a look at how those were emitted, I wasn't familiar with it until now. That's certainly a way of approaching this implementation as well, though I think the full precision info should still be in the AST somewhere rather than implied from the operand types until CodeGen. > > >> It still feels like this just introduces inconsistencies into the form of >> the AST. If we do add this extra type object to the BO, won't people wonder >> why we use ImplicitCastExpr for non-fixedpoint operations but use the >> special `QualTypeOrFPSemantics BinaryOperator::ComputationType;` for >> fixedpoint operations even though they both have nearly the same semantic >> meaning (converting to a common type before doing the operation)? > > > > 1. You would have an inconsistency in either case, since e.g. numeric + > otherwise always returns the same type as its operands, but this would not. True, but the CompoundAssignOperator already has that inconsistency with ComputationResultType. > 2. The question is easily answered by pointing at the language spec. The > language does not say that the operands are promoted to a common type; it > says the result is determined numerically from the true numeric values of the > operands. I guess. I just see that as a behavioral specification and not necessarily an implementation detail. It's perfectly acceptable to implement it as promotion to a common type (obviously, as that's what we are going to do), and I don't really see this as the spec putting any kind of requirement on how the implementation should be done. > > >>>> It might just be easier to store the full-precision info in BO directly. >>>> BO might be too common to warrant the size increase, though. >>>> FixedPointSemantics can probably be optimized to only take 32 bits. >>> >>> What you can definitely do is store a bit in BO saying that there's extra >>> storage for the intermediate "type". >> >> Is this similar to how ExtQuals works? How would this be implemented? > > Take a look at how `DeclRefExpr` stores its various optional components. `TrailingObjects`, then. That certainly wouldn't work if CompoundAssignOperator is still a subclass of BinaryOperator, so it would have to be folded in that case. So the implementation would be with a `TrailingObjects<BinaryOperator, QualType, FixedPointSemantics>` where it can have 2 QualTypes and 1 FixedPointSemantics, the QualTypes being subsumed from CompoundAssignOperator. Probably still quite a hefty amount of code that would have to be updated to make this change. > > >>>> As a side note, comparisons are still a bit up in the air. I don't think >>>> we came to a conclusion on whether they should be done in full precision >>>> or bitwise. The spec isn't clear. >>> >>> ...bitwise? >> >> The spec uses different wording for the arithmetic operations and comparison >> operations, and it's not entirely obvious what it means. For the arithmetic >> operators, it has the whole section on finding the full precision common >> type, but for comparisons it says: >> >>> When comparing fixed-point values with fixed-point values or integer values, >>> the values are compared directly; the values of the operands are not >>> converted before the >>> comparison is made. >> >> What 'directly' means in conjunction with 'the operands are not converted' >> is not clear. It's reasonable to assume that it either means comparing >> value-wise (so, the same as finding a common type that fits all values and >> then comparing; though this would obviously require conversion), or perhaps >> 'directly' means a bitwise (representation) comparison. The latter seems a >> bit farfetched to me, though. > > I think this is just like fixed-point arithmetic: you should do an exact > numeric comparison, but there's no formal conversion because it would have to > be a conversion to some concrete type, which could leave to (mandatory!) > inexactness when there's no common type that expresses the full range of both > operands. This is probably the correct interpretation, I just think it could have been stated a bit clearer that they pretty much do mean the same thing for the comparison operators as for the arithmetic operators. Repository: rC Clang https://reviews.llvm.org/D53738 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
