leonardchan added inline comments.
================ Comment at: include/clang/Basic/TargetInfo.h:90 + // by default. + bool SameFBits; + ---------------- rjmccall wrote: > Sorry for the extremely late review, but this really needs to be renamed. > Please remember that other compiler maintainers are not experts in the > feature that you happen to be currently implementing; they should be able to > look at a variable name and understand what it means, and honestly "fbits" > does not mean anything. `UnsignedFixedPointHasNoPadding`, maybe? Or you > could shorten it by switching the polarity. Made a new diff: https://reviews.llvm.org/D48727. Opted for renaming to PaddingOnUnsignedFixedPoint. ================ Comment at: include/clang/Driver/Options.td:899 + HelpText<"Force each unsigned fixed point type to have the same number of fractional bits as its corresponding signed type">; +def fno_same_fbits : Flag<["-"], "fno-same-fbits">, Group<f_Group>; ---------------- rjmccall wrote: > Okay, please also remember that *users* are not deeply embedded into the lore > and wisdom of your implementation group. :) This is not an acceptable > user-facing option name; please rename it to something with "fixed-point" in > the name. > > Also, if this is just for testing (it seems a bit early to decide that users > will need an ability to override their target about this?), maybe it should > just be a -cc1 option. Renamed to `-fpadding-on-unsigned-fixed-point` in https://reviews.llvm.org/D48727 Repository: rC Clang https://reviews.llvm.org/D46915 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
