[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: lib/Basic/TargetInfo.cpp:45
+  AccumWidth = AccumAlign = 32;
+  LongAccumWidth = LongAccumAlign = 64;
   SuitableAlign = 64;
----------------
leonardchan wrote:
> leonardchan wrote:
> > ebevhan wrote:
> > > leonardchan wrote:
> > > > rsmith wrote:
> > > > > jfb wrote:
> > > > > > This seems weird because Targets which don't have these values for the non-Accum versions will have .e.g. `sizeof(short) != sizeof(short _Accum)`. Is there a point in ever having `_Accum` differ in size, width, and alignment from the underlying type? If not, can you set these values after the sub-target has specified its preferences?
> > > > > I'm uncomfortable about opting all targets into this behavior with these default values; this will result in an ABI break if later a target updates these to the correct values. A per-target `AccumSupported` flag would help substantially. But this is OK for the short term while you're still working on the feature.
> > > > > 
> > > > > We'll also need the target to inform us of the number of integer and fractional bits for each such type.
> > > > The integral and fractional bits will be set in the target and is available in a later patch.
> > > > We'll also need the target to inform us of the number of integer and fractional bits for each such type.
> > > 
> > > I believe the only one that is needed is for the number of fractional bits; the number of integer bits is implied by the difference between the type width and fractional bits. I think I mention this in one of the other patches.
> > > 
> > > 
> > You're right. I was stuck in the mindset that we would be providing an integral and fractional value.
> > The integral and fractional bits will be set in the target and is available in a later patch.
> 
> I mean just the fractional bits since the integral will just be the bit width minus fractional.
You're assuming that all bits will be either integral or fractional bits (and in particular that there are no padding bits). I don't know if that's actually going to be true for all target ABIs, but I suppose it's OK to make this assumption until it's proven wrong by some actual target.


Repository:
  rC Clang

https://reviews.llvm.org/D46084