ebevhan added a comment. In https://reviews.llvm.org/D53738#1288333, @rjmccall wrote:
> 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. It might just be simpler and a bit more general to add a `DownscalingRoundsTowardZero` field to `FixedPointSemantics`, which specifies that the source value should be rounded toward zero before downscaling. Then conversion routines could handle the integer case explicitly. I'm not sure if the field would be useful for anything else, though; it has a pretty specific meaning. I think it's a bit odd to have `FixedPointSemantics` as a specialization of something else, since technically integers are a specialization of fixed-point values and not the other way around. 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
