This is an archive of the discontinued LLVM Phabricator instance.

[Fixed Point Arithmetic] Rename `-fsame-fbits` flag
ClosedPublic

Authored by leonardchan on Jun 28 2018, 8:47 AM.

Details

Summary
  • Rename the -fsame-fbits flag to -fpadding-on-unsigned-fixed-point
  • Move the flag from a driver option to a cc1 option
  • Rename the SameFBits member in TargetInfo to PaddingOnUnsignedFixedPoint
  • Updated descriptions

Diff Detail

Repository
rC Clang

Event Timeline

leonardchan created this revision.Jun 28 2018, 8:47 AM

Formatting and forgot to get rid of Group<f_Group> and Flags<CC1Option>

This all looks reasonable except that I think the interpretation is exactly backwards, no? From the documentation on the option, -fpadding-on-unsigned-fixed-point causes there to be padding, i.e. the inverse of the old SameFBits; but the default value, comments, and boolean checks throughout the code are all still using the old interpretation.

Oh, having the same number of fractional bits is what leads to unsigned types having one bit of padding, and vice versa.

If this flag is set to false, then the integral and fractional parts of the unsigned types take up the whole bit width of the underlying scaled integer, with unsigned types gaining one extra fractional bit which would be taken by the sign bit in the signed types. When this flag is true, the number of fractional bits for unsigned types changes to match the signed types, but the number of integral bits stays the same, which leads to one unused padding bit.

rjmccall accepted this revision.Jun 28 2018, 9:53 AM

Oh, having the same number of fractional bits is what leads to unsigned types having one bit of padding, and vice versa.

If this flag is set to false, then the integral and fractional parts of the unsigned types take up the whole bit width of the underlying scaled integer, with unsigned types gaining one extra fractional bit which would be taken by the sign bit in the signed types. When this flag is true, the number of fractional bits for unsigned types changes to match the signed types, but the number of integral bits stays the same, which leads to one unused padding bit.

Oh, of course. Sorry for the confusion; LGTM.

This revision is now accepted and ready to land.Jun 28 2018, 9:53 AM
This revision was automatically updated to reflect the committed changes.