rjmccall added a comment. In https://reviews.llvm.org/D53738#1287123, @ebevhan wrote:
> In https://reviews.llvm.org/D53738#1284213, @rjmccall wrote: > > > Not out of line with other features that significantly break with what's > > expressible. But the easy alternative to storing the intermediate "type" > > in the AST is to just provide a global function that can compute it on > > demand. > > > Yeah, it might just be simpler to go the route of not storing the computation > semantic in the AST, at least for now. That's fairly similar to the solution > in the patch, so I feel a bit silly for just going around in a circle. > > To make that work I think the patch needs some changes, though. There should > be a function in FixedPointSemantics to find the common full-precision > semantic between two semantics, and the `getFixedPointSemantics` in > ASTContext should be amended to take integer types (or another method should > be provided for this, but that one already exists). I think that the > `FixedPointConversions` method should also be embedded into the rest of > `UsualArithmeticConversions` as there shouldn't be any need to have it > separate. You still want to do the rvalue conversion and other promotions, > and the rules for fixed-point still fit into the arithmetic conversions, even > in the spec. > > `EmitFixedPointConversion` should be changed to take FixedPointSemantics > rather than QualType. This has a couple of downsides: > > - It can no longer handle floating point conversions. They would have to be > handled in EmitScalarConversion. > - Conversion from integer is fine, but conversion to integer cannot be > generalized away with the fixed-point semantics as they are currently, as > that kind of conversion must round towards zero. This requires a rounding > step for negative numbers before downscaling, which the current code does not > do. Is there a better way of generalizing this? `FixedPointSemantics` is free to do whatever is convenient for the representation; if it helps to make it able to explicitly represent an integral type — and then just assert that it's never in that form when it's used in certain places, I think that works. Although you might consider making a `FixedPointOrIntegralSemantics` class and then making `FixedPointSemantics` a subclass which adds nothing to the representation but semantically asserts that it's representing a fixed-point type. > All `EmitFixedPointAdd` should have to do is get the semantics of the > operands and result type, get their full-precision semantic, call > `EmitFixedPointConversion` on both operands, do the addition, and call it > again to convert back to the result value. Move as much of the conversions as > possible out of the function. > > Does all this sound reasonable? Makes sense to me. 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
