leonardchan added inline comments.

================
Comment at: lib/Basic/TargetInfo.cpp:45
+  AccumWidth = AccumAlign = 32;
+  LongAccumWidth = LongAccumAlign = 64;
   SuitableAlign = 64;
----------------
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.


Repository:
  rC Clang

https://reviews.llvm.org/D46084



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

Reply via email to