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

Leonard Chan via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu May 24 16:58:24 PDT 2018

leonardchan added inline comments.

Comment at: include/clang/Basic/TargetInfo.h:382
+    // enough bits to fit the minumum.
+    if (getIntWidth() < MinSignedAccumDataBits)
+      return getLongWidth();
ebevhan wrote:
> I'm not sure I agree with this interpretation. It's simply not correct to consider 'short' the 'underlying type' of 'short _Accum', 'int' the 'underlying type' of '_Accum', etc. They are wholly independent and should have separate settings altogether.
> Asserting/ensuring that a target has set an invalid width for its types should be done separately. This currently feels a bit like the TargetInfo is guessing.
> (For the record, the reason I'm requesting this change is because this implementation does not work for us downstream.)
You're right. They should be different types. I was stuck in the mindset that `short _Accum` should be tied to `short`, `_Accum` be tied to `int`, etc.

  rC Clang


More information about the cfe-commits mailing list