rjmccall added inline comments.
================
Comment at: lib/CodeGen/CGExprScalar.cpp:768
+ if (CGF.getContext().getTargetInfo().unsignedFixedPointTypesHavePadding()
&&
+ Ty->isUnsignedFixedPointType()) {
+ unsigned NumBits = CGF.getContext().getTypeSize(Ty);
----------------
ebevhan wrote:
> rjmccall wrote:
> > ebevhan wrote:
> > > rjmccall wrote:
> > > > Can you explain the padding thing? Why is padding uniformly present or
> > > > absent on all unsigned fixed point types on a target, and never on
> > > > signed types? Is this "low bits set" thing endian-specific?
> > > I'll give an example with a fract type. Let's say fract is 16 bits wide.
> > > That means the signed fract will have a scale of 15 (15 fractional bits)
> > > and one sign bit, for a value range of [-1.0, 1.0). The LSB is worth
> > > 2^-15 and the MSB is worth -2^0, similar to signed integers.
> > >
> > > The unsigned fract cannot be negative, but must also have a value range
> > > of 1.0. This means that either:
> > > * The scale remains the same (15), and the MSB is a padding bit. This bit
> > > would normally be the 2^0 bit, but since the range of the number cannot
> > > exceed 1, this bit will always be 0. In this case, the LSB is worth 2^-15
> > > (same as the signed) and the MSB is not worth anything.
> > > * The scale is that of signed fract + 1 (16). All bits of the number are
> > > value bits, and there is no padding. The LSB is worth 2^-16 and the MSB
> > > is worth 2^-1. There is no bit with a magnitude of 2^0.
> > >
> > > These rules also apply to accum types, since the number of integral bits
> > > in the unsigned accum types may not exceed that of the signed ones.
> > > Therefore, we either have to choose between having the same number of
> > > fractional bits as the signed ones (leaving the MSB as 0/padding) or
> > > having one more fractional bit than the signed ones.
> > >
> > > The lowBits is just there to construct a mask to block out the MSB. There
> > > should be no endian consideration.
> > > I'll give an example with a fract type. Let's say fract is 16 bits wide.
> > > That means the signed fract will have a scale of 15 (15 fractional bits)
> > > and one sign bit, for a value range of [-1.0, 1.0). The LSB is worth
> > > 2^-15 and the MSB is worth -2^0, similar to signed integers.
> >
> > Ah, I see, and now I've found where this is laid out in the spec, as well
> > as where the spec leaves this representation question open for unsigned
> > types.
> >
> > Is this review debating whether or not to use a padded implementation? Do
> > we not have any requirement to maintain compatibility with other compilers?
> > Do other compilers differ on the representation, perhaps by target, or is
> > there general agreement?
> >
> > In the abstract, I think it would be better to fully use all the bits
> > available, even if it means conversions are non-trivial.
> We had some discussions about representation in earlier patches. The target
> specification was a lot more free in the first revisions, but we locked it
> down to fewer parameters (scale options for accum types, fract scale being
> width-1, and the unsigned-signed scale flag) to make the implementation
> simpler.
>
> I argued for adding the target option to set the unsigned scale to the same
> as the signed; Leonard's original design did not have it. The reason is
> simply that we use that representation downstream in our DSP-C
> implementation, and things would be harder to maintain for us if the upstream
> implementation did not support it. The only other Embedded-C implementation I
> know of is the one in avr-gcc, and I believe it uses all the bits for
> unsigned.
>
> I've given some arguments for why our representation is better from an
> implementation perspective (conversion is easier, precision ranges are more
> uniform, hardware operations that operate on signed can be reused for
> unsigned) in previous patches, but I suspect that using all bits is a
> reasonable default.
I'd be happy to read those discussions instead of asking you to summarize them
if you wouldn't mind digging out a link.
That said, I agree that the right approach for Clang as a project is to allow
the target to decide. If there were really an arbitrary number of
implementations, that would be different, but from the spec it seems clear that
there are exactly two reasonable choices.
Repository:
rC Clang
https://reviews.llvm.org/D48456
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
