leonardchan added inline comments.

================
Comment at: lib/Sema/SemaExpr.cpp:1248
+  bool RHSFixed = RHSType->isFixedPointType();
+
+  if (LHSFixed && RHSFixed) {
----------------
ebevhan wrote:
> ebevhan wrote:
> > leonardchan wrote:
> > > leonardchan wrote:
> > > > ebevhan wrote:
> > > > > ebevhan wrote:
> > > > > > leonardchan wrote:
> > > > > > > ebevhan wrote:
> > > > > > > > leonardchan wrote:
> > > > > > > > > ebevhan wrote:
> > > > > > > > > > I don't see how these semantics work properly. The 
> > > > > > > > > > specification requires that operations be done in the full 
> > > > > > > > > > precision of both types. You cannot convert the types 
> > > > > > > > > > before performing the operation like this, since the 
> > > > > > > > > > operation will not be done in full precision in that case.
> > > > > > > > > > 
> > > > > > > > > > The operator semantics of Embedded-C require the operand 
> > > > > > > > > > types of binary operators to be different. It's only when 
> > > > > > > > > > you've performed the operation that you are allowed to 
> > > > > > > > > > convert the result to the resulting type.
> > > > > > > > > Initially the idea was to convert both sides to fixed point 
> > > > > > > > > types, then perform standard binary operations between the 
> > > > > > > > > fixed point types.
> > > > > > > > > 
> > > > > > > > > For the example, a `fract * int` would have the int converted 
> > > > > > > > > to a fixed point type by left shifting it by the scale of the 
> > > > > > > > > fract, multiplying, then right shifting by the scale again to 
> > > > > > > > > get the resulting fract. The only unhandled thing is 
> > > > > > > > > overflow, but the precision of the fract remains the same. 
> > > > > > > > > The operands would also be casted up beforehand so there was 
> > > > > > > > > enough space to store the result, which was casted down back 
> > > > > > > > > to the original fract after performing the right shift by the 
> > > > > > > > > scale.
> > > > > > > > > 
> > > > > > > > > Operations between fixed point types would follow a similar 
> > > > > > > > > process of casting both operands to the higher rank fixed 
> > > > > > > > > point type, and depending on the operation, more underlying 
> > > > > > > > > shifting and casting would be done to retain full precision 
> > > > > > > > > of the higher ranked type.
> > > > > > > > > 
> > > > > > > > > Though I will admit that I did not realize until now that 
> > > > > > > > > multiplying a fixed point type by an integer does not require 
> > > > > > > > > shifting the integer.
> > > > > > > > I see how you've reasoned; this is how C normally works. The 
> > > > > > > > `fract` is of higher rank than `int` and therefore is the 
> > > > > > > > 'common type' of the operation. However, even though it is 
> > > > > > > > higher rank there is no guarantee that you can perform the 
> > > > > > > > operation without overflowing. And overflow matters here; the 
> > > > > > > > spec says that it must be done in the full precision (integral 
> > > > > > > > + fractional) of both types.
> > > > > > > > 
> > > > > > > > > The only unhandled thing is overflow, but the precision of 
> > > > > > > > > the fract remains the same. The operands would also be casted 
> > > > > > > > > up beforehand so there was enough space to store the result, 
> > > > > > > > > which was casted down back to the original fract after 
> > > > > > > > > performing the right shift by the scale.
> > > > > > > > 
> > > > > > > > The precision remains the same (and while it doesn't have to be 
> > > > > > > > the same to perform an operation, it makes the implementation 
> > > > > > > > more regular; things like addition and subtraction 'just 
> > > > > > > > work'), but you cannot perform a conversion to `fract` *before* 
> > > > > > > > the operation itself, since if you do, there's nothing to 'cast 
> > > > > > > > up'. Casting up is needed for things like `fract * fract` to 
> > > > > > > > prevent overflow, but for `fract * int` you need to cast to a 
> > > > > > > > type that can fit both all values of the int and all values of 
> > > > > > > > the fract, and *then* you can cast up before doing the 
> > > > > > > > multiplication.
> > > > > > > > 
> > > > > > > > > Operations between fixed point types would follow a similar 
> > > > > > > > > process of casting both operands to the higher rank fixed 
> > > > > > > > > point type, and depending on the operation, more underlying 
> > > > > > > > > shifting and casting would be done to retain full precision 
> > > > > > > > > of the higher ranked type.
> > > > > > > > 
> > > > > > > > This might work, but I feel there could be edge cases. The E-C 
> > > > > > > > fixed-point ranks are very odd as they don't reflect reality; 
> > > > > > > > `short _Accum` cannot be considered strictly 'above' `long 
> > > > > > > > _Fract`, but the former has a higher rank than the latter. 
> > > > > > > > Depending on how the types are specified for a target, implicit 
> > > > > > > > casts between fixed-point types might inadvertantly discard 
> > > > > > > > bits, even though the spec says that operations must be done in 
> > > > > > > > full precision.
> > > > > > > I see, so just to confirm, something like a `fract * int` would 
> > > > > > > not result in any implicit casting between either operand, but 
> > > > > > > any special arithmetic, like intermediate storage types or 
> > > > > > > saturation handling, would be handled by the underlying IR?
> > > > > > > 
> > > > > > > So should really no conversions/implicit type casting should be 
> > > > > > > performed here and instead all handling of arithmetic operations 
> > > > > > > should happen somewhere during the codegen stage?
> > > > > > > 
> > > > > > > I see, so just to confirm, something like a fract * int would not 
> > > > > > > result in any implicit casting between either operand, but any 
> > > > > > > special arithmetic, like intermediate storage types or saturation 
> > > > > > > handling, would be handled by the underlying IR?
> > > > > > 
> > > > > > Yes, for operations which require precision that cannot be provided 
> > > > > > by any of the existing types, there must be an 'invisible' implicit 
> > > > > > conversion to a type which can represent all of the values of 
> > > > > > either operand. This conversion cannot be represented in the AST as 
> > > > > > it is today.
> > > > > > 
> > > > > > The simplest solution is indeed to not have any implicit cast at 
> > > > > > all in the AST and resolve these conversions when needed (CodeGen 
> > > > > > and consteval are the locations I can think of), but ultimately it 
> > > > > > feels a bit dirty... I think that the best solution AST-wise is to 
> > > > > > define a completely new type class (perhaps 
> > > > > > FullPrecisionFixedPointType) that represents a fixed-point type 
> > > > > > with arbitrary width, scale, signedness and saturation. Then you 
> > > > > > can define ImplicitCasts to an instance of this type that can fit 
> > > > > > both the `int` and the `fract`. I don't know if adding this is 
> > > > > > acceptable upstream, though.
> > > > > > 
> > > > > > I think all of these rules must apply to fixed-fixed operations as 
> > > > > > well; a `short accum * long fract` must be done as a type that does 
> > > > > > not exist, similar to fixed-int. It's not clear how saturation 
> > > > > > should work here either...
> > > > > > 
> > > > > > I also noticed now that the spec says in regards to comparison 
> > > > > > operators, `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.` I'm not sure what this means.
> > > > > In any case, to clarify, I think there are two paths to consider. 
> > > > > Either:
> > > > > 
> > > > >   - Add a new type class to the type system that encapsulates an 
> > > > > arbitrary-precision fixed-point type that can be used for implicit 
> > > > > casts when operating on fixed-point and integer types. This is in my 
> > > > > opinion the cleaner solution, since it retains invariants on the 
> > > > > types of operators and simplifies any logic that deals with 
> > > > > operators; or,
> > > > >   - Leave the operands of these operations uncasted. This is in some 
> > > > > way simpler, since it doesn't require adding a whole new type, but it 
> > > > > complicates other parts of the code. Anything that wants to deal with 
> > > > > fixed-point operators will need to know how to do fixed-point 
> > > > > conversion as well, which isn't a very good separation of 
> > > > > responsibility IMO. It also breaks the C invariant of operands of 
> > > > > arithmetic types being in a common type, which might be surprising to 
> > > > > people.
> > > > > 
> > > > > 
> > > > I'm actually more of a fan for the second case. Aside, aside from the 
> > > > literal parsing in NumericLieralParser, wouldn't the only other place 
> > > > that would actually need to know about fixed point conversion be 
> > > > `ScalarExprEmitter` under CodeGen/CGExprScalar.cpp?
> > > > 
> > > > It seems that it's this class that creates the binary operations and 
> > > > other code gen classes like CodeGenFunction just make underlying calls 
> > > > to ScalarExprEmitter, so the actual conversion logic may just be 
> > > > contained here. Most of the implicit casting handled under 
> > > > UsualArithmeticConversions seems to be handled by `VisitCastExpr` under 
> > > > ScalarExprEmitter also, so adding another casting type would in the end 
> > > > just result in another case in the switch statement there, which in 
> > > > turn may just result in another call to ScalarExprEmitter.
> > > > 
> > > > I can see how it might be weird at first that these types don't fall 
> > > > under usual arithmetic, but the standard does specify that it wouldn't.
> > > Regarding comparison operators, my guess is that it means during 
> > > comparison operations specifically, the actual underlying values of each 
> > > operand are compared instead of having the special type conversions take 
> > > place. That is, `1.0k != 1` but `1.0k == 128` (assuming scale of 7). If 
> > > this is the case, we could actually save a few operations not having to 
> > > do a shift on the integer.
> > > 
> > > I also can't seem to find a test case used by GCC  where they explicitly 
> > > compare a fixed point type against an integer. Normally, they instead 
> > > assign the FP literal to an integral type, then compare that against 
> > > another integer.
> > > 
> > > I'm referring to `CONV_ACCUM_INT` in
> > > https://github.com/gcc-mirror/gcc/blob/e11be3ea01eaf8acd8cd86d3f9c427621b64e6b4/gcc/testsuite/gcc.dg/fixed-point/convert.h
> > > I'm actually more of a fan for the second case. Aside, aside from the 
> > > literal parsing in NumericLieralParser, wouldn't the only other place 
> > > that would actually need to know about fixed point conversion be 
> > > ScalarExprEmitter under CodeGen/CGExprScalar.cpp?
> > 
> > ExprConstant (consteval) would also have to know, since the input 
> > expressions would be these 'unbalanced' binary operations. I'm not sure why 
> > it would affect literal parsing, though?
> > 
> > Regarding VisitCastExpr; in the first case, I'm not talking about adding a 
> > new CastKind, I'm talking about adding a whole new type altogether. This 
> > type would be just as much a fixed-point type as the builtin ones, just 
> > with a configurable width and scale. Then, something like this:
> > ```
> >   int * fract
> > ```
> > where int is 32 bits and fract is 16 bits Q15, would become
> > ```
> >  (fract)((FullPrecFixedPoint<32+16, 0+15>)int * (FullPrecFixedPoint<32+16, 
> > 0+15>)fract)
> > ```
> > The cast on the `int` is a `CK_IntegerToFixedPointCast`, and the cast on 
> > the `fract` is a `CK_FixedPointCast`. All values and operations are 
> > self-consistent and fully representable in the AST. Converting to and from 
> > a FullPrecFixedPoint type is no different from converting to and from, say, 
> > `fract`. They are both fixed-point types with width and scale; one is just 
> > built-in and the other is 'artificial'. The multiplication is performed 
> > like any other fixed-point operation, just in a higher width and (possibly 
> > higher) scale than either of the operands.
> > 
> > The issue I have with the second case is that the AST is somehow left 
> > 'unfinished'. There *are* casts there, but they are just not representable 
> > in the AST. In order to represent them, you would need to add these 
> > arbitrary-precision types.
> > Regarding comparison operators, my guess is that it means during comparison 
> > operations specifically, the actual underlying values of each operand are 
> > compared instead of having the special type conversions take place. That 
> > is, 1.0k != 1 but 1.0k == 128 (assuming scale of 7). If this is the case, 
> > we could actually save a few operations not having to do a shift on the 
> > integer.
> 
> Right... That seems incredibly dangerous to me; I really hope this isn't what 
> the spec means. 1.0 is by no means the same thing as 128. On top of that, it 
> means that comparisons between fixed-point and integer can vary depending on 
> the scale of the fixed-point type; this feels really shaky to me. Heck, if 
> SameFBits is false, for a scale of 7, `0.5r == 64`, but `0.5ur != 64`. It 
> might even be the case that `0.5r != 0.5ur`. Absolutely bizarre, and 
> incredibly confusing for programmers!
> 
> It might have been done this way to make it easy to inspect the raw bits of a 
> fixed-point number, but why not just do a bit-preserving conversion and 
> compare as an integer in that case?
> 
> DSP-C simply prohibits 'ambiguous' type conversions such as these to prevent 
> this confusion from happening.
I also noticed that the doc provides a `bitsfx` function for getting this 
underlying integer value representation from a fixed point type. Still going to 
leave the comparisons as what I initially interpreted them as by comparing them 
normally (`1.0k == 1`).


================
Comment at: lib/Sema/SemaExpr.cpp:1248
+  bool RHSFixed = RHSType->isFixedPointType();
+
+  if (LHSFixed && RHSFixed) {
----------------
leonardchan wrote:
> ebevhan wrote:
> > ebevhan wrote:
> > > leonardchan wrote:
> > > > leonardchan wrote:
> > > > > ebevhan wrote:
> > > > > > ebevhan wrote:
> > > > > > > leonardchan wrote:
> > > > > > > > ebevhan wrote:
> > > > > > > > > leonardchan wrote:
> > > > > > > > > > ebevhan wrote:
> > > > > > > > > > > I don't see how these semantics work properly. The 
> > > > > > > > > > > specification requires that operations be done in the 
> > > > > > > > > > > full precision of both types. You cannot convert the 
> > > > > > > > > > > types before performing the operation like this, since 
> > > > > > > > > > > the operation will not be done in full precision in that 
> > > > > > > > > > > case.
> > > > > > > > > > > 
> > > > > > > > > > > The operator semantics of Embedded-C require the operand 
> > > > > > > > > > > types of binary operators to be different. It's only when 
> > > > > > > > > > > you've performed the operation that you are allowed to 
> > > > > > > > > > > convert the result to the resulting type.
> > > > > > > > > > Initially the idea was to convert both sides to fixed point 
> > > > > > > > > > types, then perform standard binary operations between the 
> > > > > > > > > > fixed point types.
> > > > > > > > > > 
> > > > > > > > > > For the example, a `fract * int` would have the int 
> > > > > > > > > > converted to a fixed point type by left shifting it by the 
> > > > > > > > > > scale of the fract, multiplying, then right shifting by the 
> > > > > > > > > > scale again to get the resulting fract. The only unhandled 
> > > > > > > > > > thing is overflow, but the precision of the fract remains 
> > > > > > > > > > the same. The operands would also be casted up beforehand 
> > > > > > > > > > so there was enough space to store the result, which was 
> > > > > > > > > > casted down back to the original fract after performing the 
> > > > > > > > > > right shift by the scale.
> > > > > > > > > > 
> > > > > > > > > > Operations between fixed point types would follow a similar 
> > > > > > > > > > process of casting both operands to the higher rank fixed 
> > > > > > > > > > point type, and depending on the operation, more underlying 
> > > > > > > > > > shifting and casting would be done to retain full precision 
> > > > > > > > > > of the higher ranked type.
> > > > > > > > > > 
> > > > > > > > > > Though I will admit that I did not realize until now that 
> > > > > > > > > > multiplying a fixed point type by an integer does not 
> > > > > > > > > > require shifting the integer.
> > > > > > > > > I see how you've reasoned; this is how C normally works. The 
> > > > > > > > > `fract` is of higher rank than `int` and therefore is the 
> > > > > > > > > 'common type' of the operation. However, even though it is 
> > > > > > > > > higher rank there is no guarantee that you can perform the 
> > > > > > > > > operation without overflowing. And overflow matters here; the 
> > > > > > > > > spec says that it must be done in the full precision 
> > > > > > > > > (integral + fractional) of both types.
> > > > > > > > > 
> > > > > > > > > > The only unhandled thing is overflow, but the precision of 
> > > > > > > > > > the fract remains the same. The operands would also be 
> > > > > > > > > > casted up beforehand so there was enough space to store the 
> > > > > > > > > > result, which was casted down back to the original fract 
> > > > > > > > > > after performing the right shift by the scale.
> > > > > > > > > 
> > > > > > > > > The precision remains the same (and while it doesn't have to 
> > > > > > > > > be the same to perform an operation, it makes the 
> > > > > > > > > implementation more regular; things like addition and 
> > > > > > > > > subtraction 'just work'), but you cannot perform a conversion 
> > > > > > > > > to `fract` *before* the operation itself, since if you do, 
> > > > > > > > > there's nothing to 'cast up'. Casting up is needed for things 
> > > > > > > > > like `fract * fract` to prevent overflow, but for `fract * 
> > > > > > > > > int` you need to cast to a type that can fit both all values 
> > > > > > > > > of the int and all values of the fract, and *then* you can 
> > > > > > > > > cast up before doing the multiplication.
> > > > > > > > > 
> > > > > > > > > > Operations between fixed point types would follow a similar 
> > > > > > > > > > process of casting both operands to the higher rank fixed 
> > > > > > > > > > point type, and depending on the operation, more underlying 
> > > > > > > > > > shifting and casting would be done to retain full precision 
> > > > > > > > > > of the higher ranked type.
> > > > > > > > > 
> > > > > > > > > This might work, but I feel there could be edge cases. The 
> > > > > > > > > E-C fixed-point ranks are very odd as they don't reflect 
> > > > > > > > > reality; `short _Accum` cannot be considered strictly 'above' 
> > > > > > > > > `long _Fract`, but the former has a higher rank than the 
> > > > > > > > > latter. Depending on how the types are specified for a 
> > > > > > > > > target, implicit casts between fixed-point types might 
> > > > > > > > > inadvertantly discard bits, even though the spec says that 
> > > > > > > > > operations must be done in full precision.
> > > > > > > > I see, so just to confirm, something like a `fract * int` would 
> > > > > > > > not result in any implicit casting between either operand, but 
> > > > > > > > any special arithmetic, like intermediate storage types or 
> > > > > > > > saturation handling, would be handled by the underlying IR?
> > > > > > > > 
> > > > > > > > So should really no conversions/implicit type casting should be 
> > > > > > > > performed here and instead all handling of arithmetic 
> > > > > > > > operations should happen somewhere during the codegen stage?
> > > > > > > > 
> > > > > > > > I see, so just to confirm, something like a fract * int would 
> > > > > > > > not result in any implicit casting between either operand, but 
> > > > > > > > any special arithmetic, like intermediate storage types or 
> > > > > > > > saturation handling, would be handled by the underlying IR?
> > > > > > > 
> > > > > > > Yes, for operations which require precision that cannot be 
> > > > > > > provided by any of the existing types, there must be an 
> > > > > > > 'invisible' implicit conversion to a type which can represent all 
> > > > > > > of the values of either operand. This conversion cannot be 
> > > > > > > represented in the AST as it is today.
> > > > > > > 
> > > > > > > The simplest solution is indeed to not have any implicit cast at 
> > > > > > > all in the AST and resolve these conversions when needed (CodeGen 
> > > > > > > and consteval are the locations I can think of), but ultimately 
> > > > > > > it feels a bit dirty... I think that the best solution AST-wise 
> > > > > > > is to define a completely new type class (perhaps 
> > > > > > > FullPrecisionFixedPointType) that represents a fixed-point type 
> > > > > > > with arbitrary width, scale, signedness and saturation. Then you 
> > > > > > > can define ImplicitCasts to an instance of this type that can fit 
> > > > > > > both the `int` and the `fract`. I don't know if adding this is 
> > > > > > > acceptable upstream, though.
> > > > > > > 
> > > > > > > I think all of these rules must apply to fixed-fixed operations 
> > > > > > > as well; a `short accum * long fract` must be done as a type that 
> > > > > > > does not exist, similar to fixed-int. It's not clear how 
> > > > > > > saturation should work here either...
> > > > > > > 
> > > > > > > I also noticed now that the spec says in regards to comparison 
> > > > > > > operators, `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.` I'm not sure what this means.
> > > > > > In any case, to clarify, I think there are two paths to consider. 
> > > > > > Either:
> > > > > > 
> > > > > >   - Add a new type class to the type system that encapsulates an 
> > > > > > arbitrary-precision fixed-point type that can be used for implicit 
> > > > > > casts when operating on fixed-point and integer types. This is in 
> > > > > > my opinion the cleaner solution, since it retains invariants on the 
> > > > > > types of operators and simplifies any logic that deals with 
> > > > > > operators; or,
> > > > > >   - Leave the operands of these operations uncasted. This is in 
> > > > > > some way simpler, since it doesn't require adding a whole new type, 
> > > > > > but it complicates other parts of the code. Anything that wants to 
> > > > > > deal with fixed-point operators will need to know how to do 
> > > > > > fixed-point conversion as well, which isn't a very good separation 
> > > > > > of responsibility IMO. It also breaks the C invariant of operands 
> > > > > > of arithmetic types being in a common type, which might be 
> > > > > > surprising to people.
> > > > > > 
> > > > > > 
> > > > > I'm actually more of a fan for the second case. Aside, aside from the 
> > > > > literal parsing in NumericLieralParser, wouldn't the only other place 
> > > > > that would actually need to know about fixed point conversion be 
> > > > > `ScalarExprEmitter` under CodeGen/CGExprScalar.cpp?
> > > > > 
> > > > > It seems that it's this class that creates the binary operations and 
> > > > > other code gen classes like CodeGenFunction just make underlying 
> > > > > calls to ScalarExprEmitter, so the actual conversion logic may just 
> > > > > be contained here. Most of the implicit casting handled under 
> > > > > UsualArithmeticConversions seems to be handled by `VisitCastExpr` 
> > > > > under ScalarExprEmitter also, so adding another casting type would in 
> > > > > the end just result in another case in the switch statement there, 
> > > > > which in turn may just result in another call to ScalarExprEmitter.
> > > > > 
> > > > > I can see how it might be weird at first that these types don't fall 
> > > > > under usual arithmetic, but the standard does specify that it 
> > > > > wouldn't.
> > > > Regarding comparison operators, my guess is that it means during 
> > > > comparison operations specifically, the actual underlying values of 
> > > > each operand are compared instead of having the special type 
> > > > conversions take place. That is, `1.0k != 1` but `1.0k == 128` 
> > > > (assuming scale of 7). If this is the case, we could actually save a 
> > > > few operations not having to do a shift on the integer.
> > > > 
> > > > I also can't seem to find a test case used by GCC  where they 
> > > > explicitly compare a fixed point type against an integer. Normally, 
> > > > they instead assign the FP literal to an integral type, then compare 
> > > > that against another integer.
> > > > 
> > > > I'm referring to `CONV_ACCUM_INT` in
> > > > https://github.com/gcc-mirror/gcc/blob/e11be3ea01eaf8acd8cd86d3f9c427621b64e6b4/gcc/testsuite/gcc.dg/fixed-point/convert.h
> > > > I'm actually more of a fan for the second case. Aside, aside from the 
> > > > literal parsing in NumericLieralParser, wouldn't the only other place 
> > > > that would actually need to know about fixed point conversion be 
> > > > ScalarExprEmitter under CodeGen/CGExprScalar.cpp?
> > > 
> > > ExprConstant (consteval) would also have to know, since the input 
> > > expressions would be these 'unbalanced' binary operations. I'm not sure 
> > > why it would affect literal parsing, though?
> > > 
> > > Regarding VisitCastExpr; in the first case, I'm not talking about adding 
> > > a new CastKind, I'm talking about adding a whole new type altogether. 
> > > This type would be just as much a fixed-point type as the builtin ones, 
> > > just with a configurable width and scale. Then, something like this:
> > > ```
> > >   int * fract
> > > ```
> > > where int is 32 bits and fract is 16 bits Q15, would become
> > > ```
> > >  (fract)((FullPrecFixedPoint<32+16, 0+15>)int * 
> > > (FullPrecFixedPoint<32+16, 0+15>)fract)
> > > ```
> > > The cast on the `int` is a `CK_IntegerToFixedPointCast`, and the cast on 
> > > the `fract` is a `CK_FixedPointCast`. All values and operations are 
> > > self-consistent and fully representable in the AST. Converting to and 
> > > from a FullPrecFixedPoint type is no different from converting to and 
> > > from, say, `fract`. They are both fixed-point types with width and scale; 
> > > one is just built-in and the other is 'artificial'. The multiplication is 
> > > performed like any other fixed-point operation, just in a higher width 
> > > and (possibly higher) scale than either of the operands.
> > > 
> > > The issue I have with the second case is that the AST is somehow left 
> > > 'unfinished'. There *are* casts there, but they are just not 
> > > representable in the AST. In order to represent them, you would need to 
> > > add these arbitrary-precision types.
> > > Regarding comparison operators, my guess is that it means during 
> > > comparison operations specifically, the actual underlying values of each 
> > > operand are compared instead of having the special type conversions take 
> > > place. That is, 1.0k != 1 but 1.0k == 128 (assuming scale of 7). If this 
> > > is the case, we could actually save a few operations not having to do a 
> > > shift on the integer.
> > 
> > Right... That seems incredibly dangerous to me; I really hope this isn't 
> > what the spec means. 1.0 is by no means the same thing as 128. On top of 
> > that, it means that comparisons between fixed-point and integer can vary 
> > depending on the scale of the fixed-point type; this feels really shaky to 
> > me. Heck, if SameFBits is false, for a scale of 7, `0.5r == 64`, but `0.5ur 
> > != 64`. It might even be the case that `0.5r != 0.5ur`. Absolutely bizarre, 
> > and incredibly confusing for programmers!
> > 
> > It might have been done this way to make it easy to inspect the raw bits of 
> > a fixed-point number, but why not just do a bit-preserving conversion and 
> > compare as an integer in that case?
> > 
> > DSP-C simply prohibits 'ambiguous' type conversions such as these to 
> > prevent this confusion from happening.
> I also noticed that the doc provides a `bitsfx` function for getting this 
> underlying integer value representation from a fixed point type. Still going 
> to leave the comparisons as what I initially interpreted them as by comparing 
> them normally (`1.0k == 1`).
Understandable. Initially I thought it was a new cast for converting each fixed 
point type to this intermediate type. I will add this in a new separate patch 
that addresses conversions involving fixed point types since I feel this one is 
already large enough and was meant to address just parsing fixed point 
literals. 


Repository:
  rC Clang

https://reviews.llvm.org/D46915



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to