rjmccall added a comment. In https://reviews.llvm.org/D53738#1288372, @ebevhan wrote:
> 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. Well, it's not a specialization of "integer", it's a specialization of "fixed-point or integer". It depends on how useful it ends up being to know that something statically isn't integral. 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
